From 549d5be9f86ddc8fda01a13c4837c4e9ab522a54 Mon Sep 17 00:00:00 2001 From: Savannah Manning Date: Mon, 21 Oct 2024 02:27:52 -0600 Subject: [PATCH] Add support for client reports (#801) Co-authored-by: Andrea Leopardi --- lib/sentry.ex | 3 +- lib/sentry/application.ex | 1 + lib/sentry/client.ex | 16 ++++ lib/sentry/client_report.ex | 147 +++++++++++++++++++++++++++++ lib/sentry/config.ex | 13 +++ lib/sentry/envelope.ex | 49 +++++++++- lib/sentry/transport.ex | 37 ++++++-- test/envelope_test.exs | 36 ++++++- test/sentry/client_report_test.exs | 36 +++++++ test/sentry/client_test.exs | 30 ++++++ 10 files changed, 356 insertions(+), 12 deletions(-) create mode 100644 lib/sentry/client_report.ex create mode 100644 test/sentry/client_report_test.exs diff --git a/lib/sentry.ex b/lib/sentry.ex index 6a7acbd5..71ce7b91 100644 --- a/lib/sentry.ex +++ b/lib/sentry.ex @@ -179,7 +179,7 @@ defmodule Sentry do > with `:source_code_exclude_patterns`. """ - alias Sentry.{CheckIn, Client, ClientError, Config, Event, LoggerUtils, Options} + alias Sentry.{CheckIn, Client, ClientError, ClientReport, Config, Event, LoggerUtils, Options} require Logger @@ -341,6 +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]) :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 d63921d5..c8b7ee27 100644 --- a/lib/sentry/application.ex +++ b/lib/sentry/application.ex @@ -26,6 +26,7 @@ defmodule Sentry.Application do {Registry, keys: :unique, name: Sentry.Transport.SenderRegistry}, Sentry.Sources, Sentry.Dedupe, + Sentry.ClientReport, {Sentry.Integrations.CheckInIDMappings, [ max_expected_check_in_time: diff --git a/lib/sentry/client.ex b/lib/sentry/client.ex index 1d4b3695..f6fb2a3b 100644 --- a/lib/sentry/client.ex +++ b/lib/sentry/client.ex @@ -8,6 +8,7 @@ defmodule Sentry.Client do alias Sentry.{ CheckIn, ClientError, + ClientReport, Config, Dedupe, Envelope, @@ -81,6 +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]) :unsampled :excluded -> @@ -91,6 +93,20 @@ defmodule Sentry.Client do end end + @spec send_client_report(ClientReport.t()) :: + {:ok, client_report_id :: String.t()} | {:error, ClientError.t()} + def send_client_report(%ClientReport{} = client_report) do + client = Config.client() + + # This is a "private" option, only really used in testing. + request_retries = + Application.get_env(:sentry, :request_retries, Transport.default_retries()) + + client_report + |> Envelope.from_client_report() + |> Transport.encode_and_post_envelope(client, request_retries) + end + defp sample_event(sample_rate) do cond do sample_rate == 1 -> :ok diff --git a/lib/sentry/client_report.ex b/lib/sentry/client_report.ex new file mode 100644 index 00000000..6a1a6100 --- /dev/null +++ b/lib/sentry/client_report.ex @@ -0,0 +1,147 @@ +defmodule Sentry.ClientReport do + @moduledoc """ + A struct and GenServer implementation to represent and manage **client reports** for Sentry. + + Client reports are used to provide insights into which events are being dropped 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?`. + + Refer to for more details. + + *Available since v10.8.0*. + """ + + @moduledoc since: "10.8.0" + + use GenServer + alias Sentry.{Client, Config, Envelope} + + @client_report_reasons [ + :ratelimit_backoff, + :queue_overflow, + :cache_overflow, + :network_error, + :sample_rate, + :before_send, + :event_processor, + :insufficient_data, + :backpressure, + :send_error, + :internal_sdk_error + ] + + @typedoc """ + The possible reasons of the discarded event. + """ + @typedoc since: "10.8.0" + @type reason() :: + unquote(Enum.reduce(@client_report_reasons, "e(do: unquote(&1) | unquote(&2)))) + + @typedoc """ + The struct for a **client report**. + """ + @typedoc since: "10.8.0" + @type t() :: %__MODULE__{ + timestamp: String.t() | number(), + discarded_events: [%{reason: reason(), category: String.t(), quantity: pos_integer()}] + } + + 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 +end diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index 4f45cb96..5180ba77 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -156,6 +156,16 @@ defmodule Sentry.Config do [`:jason`](https://hex.pm/packages/jason) as a dependency of your application. """ ], + send_client_reports: [ + type: :boolean, + default: true, + doc: """ + Send diagnostic client reports about discarded events, interval is set to send a report + once every 30 seconds if any discarded events exist. + See [Client Reports](https://develop.sentry.dev/sdk/client-reports/) in Sentry docs. + *Available since v10.8.0*. + """ + ], server_name: [ type: :string, doc: """ @@ -571,6 +581,9 @@ defmodule Sentry.Config do @spec dedup_events?() :: boolean() def dedup_events?, do: fetch!(:dedup_events) + @spec send_client_reports?() :: boolean() + def send_client_reports?, do: fetch!(:send_client_reports) + @spec test_mode?() :: boolean() def test_mode?, do: fetch!(:test_mode) diff --git a/lib/sentry/envelope.ex b/lib/sentry/envelope.ex index c064783c..d77003bf 100644 --- a/lib/sentry/envelope.ex +++ b/lib/sentry/envelope.ex @@ -2,11 +2,11 @@ defmodule Sentry.Envelope do @moduledoc false # https://develop.sentry.dev/sdk/envelopes/ - alias Sentry.{Attachment, CheckIn, Config, Event, UUID} + alias Sentry.{Attachment, CheckIn, ClientReport, Config, Event, UUID} @type t() :: %__MODULE__{ event_id: UUID.t(), - items: [Event.t() | Attachment.t() | CheckIn.t(), ...] + items: [Event.t() | Attachment.t() | CheckIn.t() | ClientReport.t(), ...] } @enforce_keys [:event_id, :items] @@ -34,6 +34,36 @@ defmodule Sentry.Envelope do } end + @doc """ + Creates a new envelope containing the client report. + """ + @doc since: "10.8.0" + @spec from_client_report(ClientReport.t()) :: t() + def from_client_report(%ClientReport{} = client_report) do + %__MODULE__{ + event_id: UUID.uuid4_hex(), + items: [client_report] + } + end + + @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 + @doc """ Encodes the envelope into its binary representation. @@ -60,7 +90,7 @@ defmodule Sentry.Envelope do defp item_to_binary(json_library, %Event{} = event) do case event |> Sentry.Client.render_event() |> json_library.encode() do {:ok, encoded_event} -> - header = ~s({"type": "event", "length": #{byte_size(encoded_event)}}) + header = ~s({"type":"event","length":#{byte_size(encoded_event)}}) [header, ?\n, encoded_event, ?\n] {:error, _reason} = error -> @@ -85,11 +115,22 @@ defmodule Sentry.Envelope do defp item_to_binary(json_library, %CheckIn{} = check_in) do case check_in |> CheckIn.to_map() |> json_library.encode() do {:ok, encoded_check_in} -> - header = ~s({"type": "check_in", "length": #{byte_size(encoded_check_in)}}) + header = ~s({"type":"check_in","length":#{byte_size(encoded_check_in)}}) [header, ?\n, encoded_check_in, ?\n] {:error, _reason} = error -> throw(error) end end + + defp item_to_binary(json_library, %ClientReport{} = client_report) do + case client_report |> Map.from_struct() |> json_library.encode() do + {:ok, encoded_client_report} -> + header = ~s({"type":"client_report","length":#{byte_size(encoded_client_report)}}) + [header, ?\n, encoded_client_report, ?\n] + + {:error, _reason} = error -> + throw(error) + end + end end diff --git a/lib/sentry/transport.ex b/lib/sentry/transport.ex index 76db3e78..600054e2 100644 --- a/lib/sentry/transport.ex +++ b/lib/sentry/transport.ex @@ -3,7 +3,7 @@ defmodule Sentry.Transport do # This module is exclusively responsible for encoding and POSTing envelopes to Sentry. - alias Sentry.{ClientError, Config, Envelope, LoggerUtils} + alias Sentry.{ClientError, ClientReport, Config, Envelope, LoggerUtils} @default_retries [1000, 2000, 4000, 8000] @sentry_version 5 @@ -29,18 +29,24 @@ defmodule Sentry.Transport do case Envelope.to_binary(envelope) do {:ok, body} -> {endpoint, headers} = get_endpoint_and_headers() - post_envelope_with_retries(client, endpoint, headers, body, retries) + post_envelope_with_retries(client, endpoint, headers, body, retries, envelope.items) {:error, reason} -> {:error, ClientError.new({:invalid_json, reason})} end _ = maybe_log_send_result(result, envelope.items) - result end - defp post_envelope_with_retries(client, endpoint, headers, payload, retries_left) do + defp post_envelope_with_retries( + client, + endpoint, + headers, + payload, + retries_left, + envelope_items + ) do case request(client, endpoint, headers, payload) do {:ok, id} -> {:ok, id} @@ -49,20 +55,39 @@ defmodule Sentry.Transport do # own retry. {:retry_after, delay_ms} when retries_left != [] -> Process.sleep(delay_ms) - post_envelope_with_retries(client, endpoint, headers, payload, tl(retries_left)) + + post_envelope_with_retries( + client, + endpoint, + headers, + payload, + tl(retries_left), + envelope_items + ) {:retry_after, _delay_ms} -> + ClientReport.record_discarded_events(:ratelimit_backoff, envelope_items) {:error, ClientError.new(:too_many_retries)} {:error, _reason} when retries_left != [] -> [sleep_interval | retries_left] = retries_left Process.sleep(sleep_interval) - post_envelope_with_retries(client, endpoint, headers, payload, retries_left) + + post_envelope_with_retries( + client, + endpoint, + headers, + payload, + retries_left, + envelope_items + ) {:error, {:http, {status, headers, body}}} -> + ClientReport.record_discarded_events(:send_error, envelope_items) {:error, ClientError.server_error(status, headers, body)} {:error, reason} -> + ClientReport.record_discarded_events(:send_error, envelope_items) {:error, ClientError.new(reason)} end end diff --git a/test/envelope_test.exs b/test/envelope_test.exs index 82c5e797..c6b5e663 100644 --- a/test/envelope_test.exs +++ b/test/envelope_test.exs @@ -3,7 +3,7 @@ defmodule Sentry.EnvelopeTest do import Sentry.TestHelpers - alias Sentry.{Attachment, CheckIn, Envelope, Event} + alias Sentry.{Attachment, CheckIn, ClientReport, Envelope, Event} describe "to_binary/1" do test "encodes an envelope" do @@ -114,4 +114,38 @@ defmodule Sentry.EnvelopeTest do assert decoded_check_in["status"] == "ok" end end + + test "works with client reports" do + put_test_config(environment_name: "test") + + client_report = %ClientReport{ + timestamp: "2024-10-12T13:21:13", + discarded_events: [%{reason: :event_processor, category: "error", quantity: 1}] + } + + envelope = Envelope.from_client_report(client_report) + + assert {:ok, encoded} = Envelope.to_binary(envelope) + + assert [id_line, header_line, event_line] = String.split(encoded, "\n", trim: true) + assert %{"event_id" => _} = Jason.decode!(id_line) + assert %{"type" => "client_report", "length" => _} = Jason.decode!(header_line) + + assert {:ok, decoded_client_report} = Jason.decode(event_line) + assert decoded_client_report["timestamp"] == client_report.timestamp + + assert decoded_client_report["discarded_events"] == [ + %{"category" => "error", "reason" => "event_processor", "quantity" => 1} + ] + end + + test "returns correct data_category" do + assert Envelope.get_data_category(%Sentry.Event{ + event_id: Sentry.UUID.uuid4_hex(), + timestamp: "2024-10-12T13:21:13" + }) == "error" + + assert_raise FunctionClauseError, + fn -> Envelope.get_data_category(:completely_banana_value) end + end end diff --git a/test/sentry/client_report_test.exs b/test/sentry/client_report_test.exs new file mode 100644 index 00000000..9c4e8ccd --- /dev/null +++ b/test/sentry/client_report_test.exs @@ -0,0 +1,36 @@ +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 diff --git a/test/sentry/client_test.exs b/test/sentry/client_test.exs index cf803c2a..62285241 100644 --- a/test/sentry/client_test.exs +++ b/test/sentry/client_test.exs @@ -418,4 +418,34 @@ defmodule Sentry.ClientTest do end end end + + describe "send_client_report/1" do + test "succefully sends discarded events to Sentry" do + bypass = Bypass.open() + put_test_config(dsn: "http://public:secret@localhost:#{bypass.port}/1") + + client_report = + %Sentry.ClientReport{ + timestamp: "2024-10-15T14:22:49", + discarded_events: [ + %{reason: :event_processor, category: "error", quantity: 1} + ] + } + + Bypass.expect_once(bypass, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + assert [{_headers, client_report_body}] = decode_envelope!(body) + + assert client_report_body["discarded_events"] == [ + %{"category" => "error", "quantity" => 1, "reason" => "event_processor"} + ] + + assert client_report_body["timestamp"] == "2024-10-15T14:22:49" + + Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) + end) + + assert {:ok, "340"} = Client.send_client_report(client_report) + end + end end