diff --git a/lib/sentry.ex b/lib/sentry.ex index 71ce7b91..4dbfd993 100644 --- a/lib/sentry.ex +++ b/lib/sentry.ex @@ -341,7 +341,7 @@ defmodule Sentry do cond do is_nil(event.message) and event.exception == [] -> LoggerUtils.log("Cannot report event without message or exception: #{inspect(event)}") - ClientReport.record_discarded_events(:event_processor, [event]) + ClientReport.Sender.record_discarded_events(:event_processor, [event]) :ignored # If we're in test mode, let's send the event down the pipeline anyway. diff --git a/lib/sentry/application.ex b/lib/sentry/application.ex index c8b7ee27..e592ae04 100644 --- a/lib/sentry/application.ex +++ b/lib/sentry/application.ex @@ -26,7 +26,7 @@ defmodule Sentry.Application do {Registry, keys: :unique, name: Sentry.Transport.SenderRegistry}, Sentry.Sources, Sentry.Dedupe, - Sentry.ClientReport, + Sentry.ClientReport.Sender, {Sentry.Integrations.CheckInIDMappings, [ max_expected_check_in_time: diff --git a/lib/sentry/client.ex b/lib/sentry/client.ex index f6fb2a3b..87c124fc 100644 --- a/lib/sentry/client.ex +++ b/lib/sentry/client.ex @@ -82,7 +82,7 @@ defmodule Sentry.Client do :unsampled -> # See https://github.com/getsentry/develop/pull/551/files Sentry.put_last_event_id_and_source(event.event_id, event.source) - ClientReport.record_discarded_events(:sample_rate, [event]) + ClientReport.Sender.record_discarded_events(:sample_rate, [event]) :unsampled :excluded -> diff --git a/lib/sentry/client_report.ex b/lib/sentry/client_report.ex index 6a1a6100..4552b010 100644 --- a/lib/sentry/client_report.ex +++ b/lib/sentry/client_report.ex @@ -1,12 +1,13 @@ defmodule Sentry.ClientReport do @moduledoc """ - A struct and GenServer implementation to represent and manage **client reports** for Sentry. + This module represents the data structure for a **client report**. - Client reports are used to provide insights into which events are being dropped and for what reason. + Client reports are used to provide insights into which events are being + dropped (that is, not sent to Sentry) and for what reason. This module is responsible for recording, storing, and periodically sending these client reports to Sentry. You can choose to turn off these reports by configuring the - option `send_client_reports?`. + `:send_client_reports` option. Refer to for more details. @@ -15,9 +16,6 @@ defmodule Sentry.ClientReport do @moduledoc since: "10.8.0" - use GenServer - alias Sentry.{Client, Config, Envelope} - @client_report_reasons [ :ratelimit_backoff, :queue_overflow, @@ -48,100 +46,10 @@ defmodule Sentry.ClientReport do discarded_events: [%{reason: reason(), category: String.t(), quantity: pos_integer()}] } + @enforce_keys [:timestamp, :discarded_events] defstruct [:timestamp, discarded_events: %{}] - @send_interval 30_000 - - @doc false - @spec start_link([]) :: GenServer.on_start() - def start_link(opts \\ []) do - GenServer.start_link(__MODULE__, %{}, name: Keyword.get(opts, :name, __MODULE__)) - end - - @doc false - @spec record_discarded_events( - reason(), - [item] - ) :: :ok - when item: - Sentry.Attachment.t() - | Sentry.CheckIn.t() - | Sentry.ClientReport.t() - | Sentry.Event.t() - def record_discarded_events(reason, event_items, genserver \\ __MODULE__) - when is_list(event_items) do - if Enum.member?(@client_report_reasons, reason) do - _ = - event_items - |> Enum.each( - &GenServer.cast( - genserver, - {:record_discarded_events, reason, Envelope.get_data_category(&1)} - ) - ) - end - - # We silently ignore events whose reasons aren't valid because we have to add it to the allowlist in Snuba - # https://develop.sentry.dev/sdk/client-reports/ - - :ok - end - - @doc false - @impl true - def init(state) do - schedule_report() - {:ok, state} - end - - @doc false - @impl true - def handle_cast({:record_discarded_events, reason, category}, discarded_events) do - {:noreply, Map.update(discarded_events, {reason, category}, 1, &(&1 + 1))} - end - @doc false - @impl true - def handle_info(:send_report, discarded_events) do - if map_size(discarded_events) != 0 do - discarded_events = - discarded_events - |> Enum.map(fn {{reason, category}, quantity} -> - %{ - reason: reason, - category: category, - quantity: quantity - } - end) - - client_report = - %__MODULE__{ - timestamp: timestamp(), - discarded_events: discarded_events - } - - _ = - if Config.dsn() != nil && Config.send_client_reports?() do - Client.send_client_report(client_report) - end - - schedule_report() - {:noreply, %{}} - else - # state is nil so nothing to send but keep looping - schedule_report() - {:noreply, %{}} - end - end - - defp schedule_report do - Process.send_after(self(), :send_report, @send_interval) - end - - defp timestamp do - DateTime.utc_now() - |> DateTime.truncate(:second) - |> DateTime.to_iso8601() - |> String.trim_trailing("Z") - end + @spec reasons() :: [reason(), ...] + def reasons, do: @client_report_reasons end diff --git a/lib/sentry/client_report/sender.ex b/lib/sentry/client_report/sender.ex new file mode 100644 index 00000000..7ca0c6fa --- /dev/null +++ b/lib/sentry/client_report/sender.ex @@ -0,0 +1,87 @@ +defmodule Sentry.ClientReport.Sender do + @moduledoc false + + # This module is responsible for storing client reports and periodically "flushing" + # them to Sentry. + + use GenServer + + alias Sentry.{Client, ClientReport, Config, Envelope} + + @send_interval 30_000 + + @client_report_reasons ClientReport.reasons() + + @spec start_link([]) :: GenServer.on_start() + def start_link(opts \\ []) do + GenServer.start_link(__MODULE__, nil, name: Keyword.get(opts, :name, __MODULE__)) + end + + @spec record_discarded_events(atom(), [item], GenServer.server()) :: :ok + when item: + Sentry.Attachment.t() + | Sentry.CheckIn.t() + | ClientReport.t() + | Sentry.Event.t() + def record_discarded_events(reason, event_items, genserver \\ __MODULE__) + when is_list(event_items) do + # We silently ignore events whose reasons aren't valid because we have to add it to the allowlist in Snuba + # https://develop.sentry.dev/sdk/client-reports/ + if Enum.member?(@client_report_reasons, reason) do + Enum.each( + event_items, + fn item -> + GenServer.cast( + genserver, + {:record_discarded_events, reason, Envelope.get_data_category(item)} + ) + end + ) + end + end + + ## Callbacks + + @impl true + def init(nil) do + schedule_report() + {:ok, _state = %{}} + end + + @impl true + def handle_cast({:record_discarded_events, reason, category}, discarded_events) do + {:noreply, Map.update(discarded_events, {reason, category}, 1, &(&1 + 1))} + end + + @impl true + def handle_info(:send_report, state) do + _ = + if map_size(state) != 0 and Config.dsn() != nil and Config.send_client_reports?() do + client_report = + %ClientReport{ + timestamp: + DateTime.utc_now() + |> DateTime.truncate(:second) + |> DateTime.to_iso8601() + |> String.trim_trailing("Z"), + discarded_events: + Enum.map(state, fn {{reason, category}, quantity} -> + %{ + reason: reason, + category: category, + quantity: quantity + } + end) + } + + Client.send_client_report(client_report) + end + + schedule_report() + {:noreply, %{}} + end + + defp schedule_report do + Process.send_after(self(), :send_report, @send_interval) + end +end diff --git a/lib/sentry/envelope.ex b/lib/sentry/envelope.ex index d77003bf..649f6e85 100644 --- a/lib/sentry/envelope.ex +++ b/lib/sentry/envelope.ex @@ -46,23 +46,16 @@ defmodule Sentry.Envelope do } end + @doc """ + Returns the "data category" of the envelope's contents (to be used in client reports and more). + """ + @doc since: "10.8.0" @spec get_data_category(Attachment.t() | CheckIn.t() | ClientReport.t() | Event.t()) :: String.t() - def get_data_category(%mod{} = type) when mod in [Attachment, CheckIn, ClientReport, Event] do - case type do - %Attachment{} -> - "attachment" - - %CheckIn{} -> - "monitor" - - %ClientReport{} -> - "internal" - - %Event{} -> - "error" - end - end + def get_data_category(%Attachment{}), do: "attachment" + def get_data_category(%CheckIn{}), do: "monitor" + def get_data_category(%ClientReport{}), do: "internal" + def get_data_category(%Event{}), do: "error" @doc """ Encodes the envelope into its binary representation. diff --git a/lib/sentry/transport.ex b/lib/sentry/transport.ex index 600054e2..a353b2b8 100644 --- a/lib/sentry/transport.ex +++ b/lib/sentry/transport.ex @@ -66,7 +66,7 @@ defmodule Sentry.Transport do ) {:retry_after, _delay_ms} -> - ClientReport.record_discarded_events(:ratelimit_backoff, envelope_items) + ClientReport.Sender.record_discarded_events(:ratelimit_backoff, envelope_items) {:error, ClientError.new(:too_many_retries)} {:error, _reason} when retries_left != [] -> @@ -83,11 +83,11 @@ defmodule Sentry.Transport do ) {:error, {:http, {status, headers, body}}} -> - ClientReport.record_discarded_events(:send_error, envelope_items) + ClientReport.Sender.record_discarded_events(:send_error, envelope_items) {:error, ClientError.server_error(status, headers, body)} {:error, reason} -> - ClientReport.record_discarded_events(:send_error, envelope_items) + ClientReport.Sender.record_discarded_events(:send_error, envelope_items) {:error, ClientError.new(reason)} end end diff --git a/mix.exs b/mix.exs index 802fbb4c..9ab0ff2c 100644 --- a/mix.exs +++ b/mix.exs @@ -50,7 +50,7 @@ defmodule Sentry.Mixfile do groups_for_modules: [ "Plug and Phoenix": [Sentry.PlugCapture, Sentry.PlugContext, Sentry.LiveViewHook], Loggers: [Sentry.LoggerBackend, Sentry.LoggerHandler], - "Data Structures": [Sentry.Attachment, Sentry.CheckIn], + "Data Structures": [Sentry.Attachment, Sentry.CheckIn, Sentry.ClientReport], HTTP: [Sentry.HTTPClient, Sentry.HackneyClient], Interfaces: [~r/^Sentry\.Interfaces/], Testing: [Sentry.Test] diff --git a/test/sentry/client_report/sender_test.exs b/test/sentry/client_report/sender_test.exs new file mode 100644 index 00000000..8d502262 --- /dev/null +++ b/test/sentry/client_report/sender_test.exs @@ -0,0 +1,72 @@ +defmodule Sentry.ClientReportTest do + use Sentry.Case, async: false + + import Sentry.TestHelpers + + alias Sentry.ClientReport.Sender + alias Sentry.Event + + setup do + original_retries = + Application.get_env(:sentry, :request_retries, Sentry.Transport.default_retries()) + + on_exit(fn -> Application.put_env(:sentry, :request_retries, original_retries) end) + + Application.put_env(:sentry, :request_retries, []) + + bypass = Bypass.open() + put_test_config(dsn: "http://public:secret@localhost:#{bypass.port}/1") + %{bypass: bypass} + end + + describe "record_discarded_events/2 + flushing" do + test "succefully records the discarded event to the client report", %{bypass: bypass} do + start_supervised!({Sender, name: :test_client_report}) + + events = [ + %Event{ + event_id: Sentry.UUID.uuid4_hex(), + timestamp: "2024-10-12T13:21:13" + } + ] + + assert :ok = Sender.record_discarded_events(:before_send, events, :test_client_report) + + assert :sys.get_state(:test_client_report) == %{{:before_send, "error"} => 1} + + assert :ok = Sender.record_discarded_events(:before_send, events, :test_client_report) + + assert :sys.get_state(:test_client_report) == %{{:before_send, "error"} => 2} + + assert :ok = Sender.record_discarded_events(:event_processor, events, :test_client_report) + assert :ok = Sender.record_discarded_events(:network_error, events, :test_client_report) + + assert :sys.get_state(:test_client_report) == %{ + {:before_send, "error"} => 2, + {:event_processor, "error"} => 1, + {:network_error, "error"} => 1 + } + + send(Process.whereis(:test_client_report), :send_report) + + Bypass.expect(bypass, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + + assert [{%{"type" => "client_report", "length" => _}, client_report}] = + decode_envelope!(body) + + assert client_report["discarded_events"] == [ + %{"reason" => "before_send", "category" => "error", "quantity" => 2}, + %{"reason" => "event_processor", "category" => "error", "quantity" => 1}, + %{"reason" => "network_error", "category" => "error", "quantity" => 1} + ] + + assert client_report["timestamp"] =~ ~r/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}$/ + + Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) + end) + + assert :sys.get_state(:test_client_report) == %{} + end + end +end diff --git a/test/sentry/client_report_test.exs b/test/sentry/client_report_test.exs deleted file mode 100644 index 9c4e8ccd..00000000 --- a/test/sentry/client_report_test.exs +++ /dev/null @@ -1,36 +0,0 @@ -defmodule Sentry.ClientReportTest do - use Sentry.Case, async: false - - alias Sentry.{ClientReport, Event} - - describe "record_discarded_events/2" do - test "succefully records the discarded event to the client report" do - {:ok, _clientreport} = start_supervised({ClientReport, [name: :test_client_report]}) - - events = [ - %Event{ - event_id: Sentry.UUID.uuid4_hex(), - timestamp: "2024-10-12T13:21:13" - } - ] - - assert ClientReport.record_discarded_events(:before_send, events, :test_client_report) == - :ok - - assert :sys.get_state(:test_client_report) == %{{:before_send, "error"} => 1} - - ClientReport.record_discarded_events(:before_send, events, :test_client_report) - - assert :sys.get_state(:test_client_report) == %{{:before_send, "error"} => 2} - - ClientReport.record_discarded_events(:event_processor, events, :test_client_report) - ClientReport.record_discarded_events(:network_error, events, :test_client_report) - - assert :sys.get_state(:test_client_report) == %{ - {:before_send, "error"} => 2, - {:event_processor, "error"} => 1, - {:network_error, "error"} => 1 - } - end - end -end