From e0d05c437e1e12f51e77ba08e67df68da3737e30 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 4 Aug 2026 13:36:39 +0000 Subject: [PATCH 1/4] refactor(test): encapsulate rate limiter setup --- .../telemetry_processor_integration_test.exs | 74 +++---------------- test/sentry/transport/rate_limiter_test.exs | 25 +++---- test/support/test_helpers.ex | 35 +++++++++ 3 files changed, 57 insertions(+), 77 deletions(-) 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 From 67897862bfd6fcac72da7cb8dfe5b0814825d9cf Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Wed, 5 Aug 2026 11:56:34 +0000 Subject: [PATCH 2/4] fix(rate-limiting): drop only the rate-limited attachments from an envelope --- lib/sentry/transport.ex | 67 +++++++++---- lib/sentry/transport/rate_limiter.ex | 8 ++ test/sentry/transport_test.exs | 142 ++++++++++++++++++++++++++- 3 files changed, 195 insertions(+), 22 deletions(-) diff --git a/lib/sentry/transport.ex b/lib/sentry/transport.ex index fe5cb473..fff7749a 100644 --- a/lib/sentry/transport.ex +++ b/lib/sentry/transport.ex @@ -27,13 +27,22 @@ defmodule Sentry.Transport do def encode_and_post_envelope(%Envelope{} = envelope, client, retries \\ @default_retries) when is_atom(client) and is_list(retries) do result = - case Envelope.to_binary(envelope) do - {:ok, body} -> - {endpoint, headers} = get_endpoint_and_headers() - post_envelope_with_retries(client, endpoint, headers, body, retries, envelope.items) - - {:error, reason} -> - {:error, ClientError.new({:invalid_json, reason})} + case filter_rate_limited(envelope) do + {:send, envelope, discarded_items} -> + ClientReport.Sender.record_discarded_events(:ratelimit_backoff, discarded_items) + + case Envelope.to_binary(envelope) do + {:ok, body} -> + {endpoint, headers} = get_endpoint_and_headers() + post_envelope_with_retries(client, endpoint, headers, body, retries, envelope.items) + + {:error, reason} -> + {:error, ClientError.new({:invalid_json, reason})} + end + + {:drop, discarded_items} -> + ClientReport.Sender.record_discarded_events(:ratelimit_backoff, discarded_items) + {:error, ClientError.new(:rate_limited)} end _ = maybe_log_send_result(result, envelope.items) @@ -83,20 +92,39 @@ defmodule Sentry.Transport do end end - defp check_rate_limited(envelope_items) do - rate_limited? = - Enum.any?(envelope_items, fn item -> - item - |> Envelope.get_data_category() - |> RateLimiter.rate_limited_for_category?() - end) + defp filter_rate_limited(%Envelope{items: items} = envelope) do + cond do + RateLimiter.global_rate_limited?() -> + {:drop, items} + + Enum.any?(items, &whole_envelope_rate_limited?/1) -> + {:drop, items} + + true -> + {discarded_items, retained_items} = Enum.split_with(items, &attachment_rate_limited?/1) - if rate_limited?, do: {:error, :rate_limited}, else: :ok + case retained_items do + [] -> {:drop, discarded_items} + _items -> {:send, %Envelope{envelope | items: retained_items}, discarded_items} + end + end end - defp request(client, endpoint, headers, body, envelope_items) do - with :ok <- check_rate_limited(envelope_items), - {:ok, 200, _headers, body} <- + defp whole_envelope_rate_limited?(%Sentry.Attachment{}), do: false + + defp whole_envelope_rate_limited?(item) do + item + |> Envelope.get_data_category() + |> RateLimiter.rate_limited_for_category?() + end + + defp attachment_rate_limited?(item) do + match?(%Sentry.Attachment{}, item) and + RateLimiter.rate_limited_for_category?(Envelope.get_data_category(item)) + end + + defp request(client, endpoint, headers, body, _envelope_items) do + with {:ok, 200, _headers, body} <- client_post_and_validate_return_value(client, endpoint, headers, body), {:ok, json} <- Sentry.JSON.decode(body, Config.json_library()) do {:ok, Map.get(json, "id")} @@ -110,9 +138,6 @@ defmodule Sentry.Transport do {:ok, status, headers, body} -> {:error, {:http, {status, headers, body}}} - {:error, :rate_limited} -> - {:error, :rate_limited} - {:error, reason} -> {:error, {:request_failure, reason}} end diff --git a/lib/sentry/transport/rate_limiter.ex b/lib/sentry/transport/rate_limiter.ex index 16e9cff2..4a10ef72 100644 --- a/lib/sentry/transport/rate_limiter.ex +++ b/lib/sentry/transport/rate_limiter.ex @@ -90,6 +90,11 @@ defmodule Sentry.Transport.RateLimiter do rate_limited?(category, now) or rate_limited?(:global, now) end + @spec global_rate_limited?() :: boolean() + def global_rate_limited? do + rate_limited?(:global, System.system_time(:second)) + end + @doc """ Checks whether sending items of the given data category is currently limited. @@ -108,6 +113,9 @@ defmodule Sentry.Transport.RateLimiter do def rate_limited_for_category?("trace_metric"), do: rate_limited?("trace_metric") or rate_limited?("trace_metric_byte") + def rate_limited_for_category?("attachment"), + do: rate_limited?("attachment") or rate_limited?("attachment_item") + def rate_limited_for_category?(category) when is_binary(category), do: rate_limited?(category) diff --git a/test/sentry/transport_test.exs b/test/sentry/transport_test.exs index 3e5d2934..c1c11f46 100644 --- a/test/sentry/transport_test.exs +++ b/test/sentry/transport_test.exs @@ -4,7 +4,16 @@ defmodule Sentry.TransportTest do import Sentry.TestHelpers import ExUnit.CaptureLog - alias Sentry.{ClientError, Envelope, Event, FinchClient, HackneyClient, Transport} + alias Sentry.{ + Attachment, + ClientError, + ClientReport, + Envelope, + Event, + FinchClient, + HackneyClient, + Transport + } describe "encode_and_post_envelope/2" do setup do @@ -368,6 +377,137 @@ defmodule Sentry.TransportTest do Transport.encode_and_post_envelope(envelope2, HackneyClient, _retries = []) end + test "sends an event without attachments when attachments are rate limited", %{bypass: bypass} do + event = Event.create_event(message: "event with attachment") + event = %Event{event | attachments: [%Attachment{filename: "report.txt", data: "report"}]} + + envelope = Envelope.from_event(event) + set_rate_limit("attachment") + + Bypass.expect_once(bypass, "POST", "/api/1/envelope/", fn conn -> + assert {:ok, body, conn} = Plug.Conn.read_body(conn) + assert [{%{"type" => "event"}, _event}] = decode_envelope!(body) + Plug.Conn.resp(conn, 200, ~s<{"id":"event-id"}>) + end) + + assert {:ok, "event-id"} = Transport.encode_and_post_envelope(envelope, FinchClient) + end + + test "sends an event while dropping all of its rate-limited attachments", %{bypass: bypass} do + assert :ok = ClientReport.Sender.flush() + + event = Event.create_event(message: "event with attachments") + + event = + %Event{ + event + | attachments: [ + %Attachment{filename: "first.txt", data: "first"}, + %Attachment{filename: "second.txt", data: "second"} + ] + } + + envelope = Envelope.from_event(event) + set_rate_limit("attachment") + + Bypass.expect_once(bypass, "POST", "/api/1/envelope/", fn conn -> + assert {:ok, body, conn} = Plug.Conn.read_body(conn) + assert [{%{"type" => "event"}, _event}] = decode_envelope!(body) + Plug.Conn.resp(conn, 200, ~s<{"id":"event-id"}>) + end) + + assert {:ok, "event-id"} = Transport.encode_and_post_envelope(envelope, FinchClient) + assert wait_until(fn -> :sys.get_state(ClientReport.Sender) != %{} end) + + assert :sys.get_state(ClientReport.Sender) == %{ + {:ratelimit_backoff, "attachment"} => 2 + } + end + + test "sends an event without attachments when attachment items are rate limited", %{ + bypass: bypass + } do + event = Event.create_event(message: "event with attachment") + event = %Event{event | attachments: [%Attachment{filename: "report.txt", data: "report"}]} + envelope = Envelope.from_event(event) + set_rate_limit("attachment_item") + + Bypass.expect_once(bypass, "POST", "/api/1/envelope/", fn conn -> + assert {:ok, body, conn} = Plug.Conn.read_body(conn) + assert [{%{"type" => "event"}, _event}] = decode_envelope!(body) + Plug.Conn.resp(conn, 200, ~s<{"id":"event-id"}>) + end) + + assert {:ok, "event-id"} = Transport.encode_and_post_envelope(envelope, FinchClient) + end + + test "records only dropped attachments in the client report" do + assert :ok = ClientReport.Sender.flush() + + event = Event.create_event(message: "event with attachment") + attachment = %Attachment{filename: "report.txt", data: "report"} + envelope = Envelope.from_event(%Event{event | attachments: [attachment]}) + set_rate_limit("attachment") + + assert {:ok, _event_id} = + Transport.encode_and_post_envelope(envelope, FinchClient, _retries = []) + + assert wait_until(fn -> :sys.get_state(ClientReport.Sender) != %{} end) + + assert :sys.get_state(ClientReport.Sender) == %{ + {:ratelimit_backoff, "attachment"} => 1 + } + end + + test "drops an event and its attachments when the error category is rate limited" do + assert :ok = ClientReport.Sender.flush() + + event = Event.create_event(message: "event with attachment") + event = %Event{event | attachments: [%Attachment{filename: "report.txt", data: "report"}]} + envelope = Envelope.from_event(event) + set_rate_limit("error") + + assert {:error, %ClientError{reason: :rate_limited}} = + Transport.encode_and_post_envelope(envelope, FinchClient, _retries = []) + + assert wait_until(fn -> :sys.get_state(ClientReport.Sender) != %{} end) + + assert :sys.get_state(ClientReport.Sender) == %{ + {:ratelimit_backoff, "attachment"} => 1, + {:ratelimit_backoff, "error"} => 1 + } + end + + test "drops an event and its attachments when error and attachment limits are active" do + assert :ok = ClientReport.Sender.flush() + + event = Event.create_event(message: "event with attachment") + event = %Event{event | attachments: [%Attachment{filename: "report.txt", data: "report"}]} + envelope = Envelope.from_event(event) + set_rate_limit("error") + set_rate_limit("attachment") + + assert {:error, %ClientError{reason: :rate_limited}} = + Transport.encode_and_post_envelope(envelope, FinchClient, _retries = []) + + assert wait_until(fn -> :sys.get_state(ClientReport.Sender) != %{} end) + + assert :sys.get_state(ClientReport.Sender) == %{ + {:ratelimit_backoff, "attachment"} => 1, + {:ratelimit_backoff, "error"} => 1 + } + end + + test "drops an event and its attachments when a global rate limit is active" do + event = Event.create_event(message: "event with attachment") + event = %Event{event | attachments: [%Attachment{filename: "report.txt", data: "report"}]} + envelope = Envelope.from_event(event) + set_rate_limit(:global) + + assert {:error, %ClientError{reason: :rate_limited}} = + Transport.encode_and_post_envelope(envelope, FinchClient, _retries = []) + end + test "handles multiple categories in single X-Sentry-Rate-Limits header", %{bypass: bypass} do envelope = Envelope.from_event(Event.create_event(message: "Hello")) From 9fc5d91e5e1eda534e8625ccd113d6996768cca2 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Wed, 5 Aug 2026 11:56:38 +0000 Subject: [PATCH 3/4] fix(client-reports): report every buffered attachment a rate limit discards --- lib/sentry/client.ex | 7 +- .../telemetry_processor_integration_test.exs | 65 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/lib/sentry/client.ex b/lib/sentry/client.ex index 695b2893..c2c249a0 100644 --- a/lib/sentry/client.ex +++ b/lib/sentry/client.ex @@ -239,8 +239,11 @@ defmodule Sentry.Client do defp encode_and_send(%Event{} = event, _result_type = :none, client, _request_retries) do if Config.telemetry_processor_category?(:error) do case TelemetryProcessor.add(event) do - {:ok, {:rate_limited, data_category}} -> - ClientReport.Sender.record_discarded_events(:ratelimit_backoff, data_category) + {:ok, {:rate_limited, _data_category}} -> + ClientReport.Sender.record_discarded_events( + :ratelimit_backoff, + [event | event.attachments] + ) :ok -> :ok diff --git a/test/sentry/telemetry_processor_integration_test.exs b/test/sentry/telemetry_processor_integration_test.exs index 545fb11d..f6a45fec 100644 --- a/test/sentry/telemetry_processor_integration_test.exs +++ b/test/sentry/telemetry_processor_integration_test.exs @@ -477,6 +477,71 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do assert ratelimit_event["quantity"] == 1 end + test "records attachments when a global limit drops an error before buffering", ctx do + put_test_config(telemetry_processor_categories: [:error, :log]) + + error_buffer = TelemetryProcessor.get_buffer(ctx.processor, :error) + + set_rate_limit(:global) + + :ok = + Sentry.Context.add_attachment(%Sentry.Attachment{filename: "report.txt", data: "report"}) + + on_exit(&Sentry.Context.clear_attachments/0) + + Sentry.capture_message("pre-buffer-global-limit", result: :none) + + assert Buffer.size(error_buffer) == 0 + + assert collect_discarded_outcomes(ctx.ref, "ratelimit_backoff") == %{ + "attachment" => 1, + "error" => 1 + } + end + + test "sends an error without attachments when attachments are rate limited", ctx do + put_test_config(telemetry_processor_categories: [:error, :log]) + + set_rate_limit("attachment", scope: :scheduler) + + :ok = + Sentry.Context.add_attachment(%Sentry.Attachment{filename: "report.txt", data: "report"}) + + on_exit(&Sentry.Context.clear_attachments/0) + + Sentry.capture_message("pre-buffer-attachment-limit", result: :none) + + assert [[{%{"type" => "event"}, event}]] = collect_envelopes(ctx.ref, 1, timeout: 2000) + assert event["message"]["formatted"] == "pre-buffer-attachment-limit" + + assert collect_discarded_outcomes(ctx.ref, "ratelimit_backoff") == %{ + "attachment" => 1 + } + end + + test "sends an error while dropping all of its rate-limited attachments", ctx do + put_test_config(telemetry_processor_categories: [:error, :log]) + + set_rate_limit("attachment", scope: :scheduler) + + :ok = + Sentry.Context.add_attachment(%Sentry.Attachment{filename: "first.txt", data: "first"}) + + :ok = + Sentry.Context.add_attachment(%Sentry.Attachment{filename: "second.txt", data: "second"}) + + on_exit(&Sentry.Context.clear_attachments/0) + + Sentry.capture_message("pre-buffer-multiple-attachment-limit", result: :none) + + assert [[{%{"type" => "event"}, event}]] = collect_envelopes(ctx.ref, 1, timeout: 2000) + assert event["message"]["formatted"] == "pre-buffer-multiple-attachment-limit" + + assert collect_discarded_outcomes(ctx.ref, "ratelimit_backoff") == %{ + "attachment" => 2 + } + end + test "drops rate-limited check-in events before they enter the buffer", ctx do put_test_config(telemetry_processor_categories: [:check_in, :log]) From a2b6a0dae6a2dbd931b85310db5210c3974c6d3b Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Wed, 5 Aug 2026 11:56:51 +0000 Subject: [PATCH 4/4] fix(rate-limiting): gate attachments on the attachment_item limit --- lib/sentry/transport/rate_limiter.ex | 15 ++++++------- .../telemetry_processor_integration_test.exs | 20 ++++++++++++++++++ test/sentry/transport/rate_limiter_test.exs | 21 +++++++++++++++++++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/lib/sentry/transport/rate_limiter.ex b/lib/sentry/transport/rate_limiter.ex index 4a10ef72..6c01baeb 100644 --- a/lib/sentry/transport/rate_limiter.ex +++ b/lib/sentry/transport/rate_limiter.ex @@ -98,13 +98,14 @@ defmodule Sentry.Transport.RateLimiter do @doc """ Checks whether sending items of the given data category is currently limited. - Logs and metrics have a companion byte category (`log_byte` / - `trace_metric_byte`) that Sentry can limit independently of the count - category, so a limit on either one must suppress sending. Every other category - gates on itself alone. - - So an active `log_byte` limit makes this return `true` for `"log_item"`, even - though `rate_limited?("log_item")` on its own is `false`. + Logs, metrics, and attachments have companion categories (`log_byte`, + `trace_metric_byte`, and `attachment_item`) that Sentry can limit + independently of the count category, so a limit on either one must suppress + sending. Every other category gates on itself alone. + + So an active `log_byte` limit makes this return `true` for `"log_item"`, and + an active `attachment_item` limit does the same for `"attachment"`, even + though the corresponding `rate_limited?/1` call on its own is `false`. """ @spec rate_limited_for_category?(String.t()) :: boolean() def rate_limited_for_category?("log_item"), diff --git a/test/sentry/telemetry_processor_integration_test.exs b/test/sentry/telemetry_processor_integration_test.exs index f6a45fec..a4ae3fd9 100644 --- a/test/sentry/telemetry_processor_integration_test.exs +++ b/test/sentry/telemetry_processor_integration_test.exs @@ -519,6 +519,26 @@ defmodule Sentry.TelemetryProcessorIntegrationTest do } end + test "sends an error without attachments when attachment items are rate limited", ctx do + put_test_config(telemetry_processor_categories: [:error, :log]) + + set_rate_limit("attachment_item", scope: :scheduler) + + :ok = + Sentry.Context.add_attachment(%Sentry.Attachment{filename: "report.txt", data: "report"}) + + on_exit(&Sentry.Context.clear_attachments/0) + + Sentry.capture_message("pre-buffer-attachment-item-limit", result: :none) + + assert [[{%{"type" => "event"}, event}]] = collect_envelopes(ctx.ref, 1, timeout: 2000) + assert event["message"]["formatted"] == "pre-buffer-attachment-item-limit" + + assert collect_discarded_outcomes(ctx.ref, "ratelimit_backoff") == %{ + "attachment" => 1 + } + end + test "sends an error while dropping all of its rate-limited attachments", ctx do put_test_config(telemetry_processor_categories: [:error, :log]) diff --git a/test/sentry/transport/rate_limiter_test.exs b/test/sentry/transport/rate_limiter_test.exs index 1b7fd8e7..194fc6c7 100644 --- a/test/sentry/transport/rate_limiter_test.exs +++ b/test/sentry/transport/rate_limiter_test.exs @@ -87,6 +87,27 @@ defmodule Sentry.Transport.RateLimiterTest do assert RateLimiter.rate_limited?("trace_metric") == false end + test "gates attachments on the attachment limit" do + set_rate_limit("attachment") + + assert RateLimiter.rate_limited_for_category?("attachment") == true + assert RateLimiter.rate_limited?("attachment") == true + end + + test "gates attachments on the attachment_item limit" do + set_rate_limit("attachment_item") + + assert RateLimiter.rate_limited_for_category?("attachment") == true + assert RateLimiter.rate_limited?("attachment") == false + end + + test "does not gate errors on attachment limits" do + set_rate_limit("attachment") + set_rate_limit("attachment_item") + + assert RateLimiter.rate_limited_for_category?("error") == false + end + test "gates a category on itself when it has no companion byte category" do set_rate_limit("error")