Add :discard_threshold option to logger handler (#849)

Closes #848.

Co-authored-by: Andrea Leopardi <[email protected]>

Author: ananthakumaran

lib/sentry/logger_handler.ex

@@ -76,16 +76,32 @@ defmodule Sentry.LoggerHandler do
       default: nil
     ],
     sync_threshold: [
-      type: :non_neg_integer,
+      type: {:or, [nil, :non_neg_integer]},
       default: 100,
       doc: """
-      *since v10.6.0* - The number of queued events after which this handler switches
+      (*since v10.6.0*) The number of queued events after which this handler switches
       to *sync mode*. Generally, this handler sends messages to Sentry **asynchronously**,
       equivalent to using `result: :none` in `Sentry.send_event/2`. However, if the number
       of queued events exceeds this threshold, the handler will switch to *sync mode*,
       where it starts using `result: :sync` to block until the event is sent. If you always
       want to use sync mode, set this option to `0`. This option effectively implements
       **overload protection**.
+
+      If you would rather *drop events* to shed load instead, use the `:discard_threshold` option.
+      `:sync_threshold` and `:discard_threshold` cannot be used together. The default behavior
+      of the handler is to switch to sync mode, so to disable this option and discard events
+      instead set `:sync_threshold` to `nil` and set `:discard_threshold` instead.
+      """
+    ],
+    discard_threshold: [
+      type: {:or, [nil, :non_neg_integer]},
+      default: nil,
+      doc: """
+      (*since v10.9.0*) The number of queued events after which this handler will start
+      to **discard** events. This option effectively implements **load shedding**.
+
+      `:discard_threshold` and `:sync_threshold` cannot be used together. If you set this option,
+      set `:sync_threshold` to `nil`.
       """
     ]
   ]
@@ -231,7 +247,8 @@ defmodule Sentry.LoggerHandler do
     :tags_from_metadata,
     :capture_log_messages,
     :rate_limiting,
-    :sync_threshold
+    :sync_threshold,
+    :discard_threshold
   ]
 
   ## Logger handler callbacks
@@ -319,6 +336,11 @@ defmodule Sentry.LoggerHandler do
       config.rate_limiting && RateLimiter.increment(handler_id) == :rate_limited ->
         :ok
 
+      # Discard event.
+      config.discard_threshold &&
+          SenderPool.get_queued_events_counter() >= config.discard_threshold ->
+        :ok
+
       true ->
         # Logger handlers run in the process that logs, so we already read all the
         # necessary Sentry context from the process dictionary (when creating the event).
@@ -407,7 +429,14 @@ defmodule Sentry.LoggerHandler do
       |> Map.to_list()
       |> NimbleOptions.validate!(@options_schema)
 
-    struct!(existing_config, validated_config)
+    config = struct!(existing_config, validated_config)
+
+    if config.sync_threshold && config.discard_threshold do
+      raise ArgumentError,
+            ":sync_threshold and :discard_threshold cannot be used together, one of them must be nil"
+    else
+      config
+    end
   end
 
   defp log_from_crash_reason(
@@ -642,7 +671,8 @@ defmodule Sentry.LoggerHandler do
 
     defp capture(unquote(function), exception_or_message, sentry_opts, %__MODULE__{} = config) do
       sentry_opts =
-        if SenderPool.get_queued_events_counter() >= config.sync_threshold do
+        if config.sync_threshold &&
+             SenderPool.get_queued_events_counter() >= config.sync_threshold do
           Keyword.put(sentry_opts, :result, :sync)
         else
           sentry_opts

test/sentry/logger_handler_test.exs

@@ -601,14 +601,82 @@ defmodule Sentry.LoggerHandlerTest do
     end
   end
 
-  defp register_before_send(_context) do
+  describe "discard threshold" do
+    @tag handler_config: %{
+           discard_threshold: 2,
+           sync_threshold: nil,
+           capture_log_messages: true
+         },
+         send_request: true
+    test "discards logged messages", %{sender_ref: ref} do
+      register_delay()
+
+      Logger.error("First")
+      assert_receive {^ref, %{message: %{formatted: "First"}}}
+
+      Logger.error("Second")
+      assert_receive {^ref, %{message: %{formatted: "Second"}}}
+
+      Logger.error("Third")
+      refute_receive {^ref, _event}, 100
+
+      Process.sleep(300)
+      Logger.error("Fourth")
+      assert_receive {^ref, %{message: %{formatted: "Fourth"}}}
+    end
+  end
+
+  @tag handler_config: %{
+         sync_threshold: 2
+       }
+  test "cannot set discard_threshold and sync_threshold" do
+    assert {:ok, %{config: config}} = :logger.get_handler_config(@handler_name)
+
+    assert {:error,
+            {:callback_crashed,
+             {:error,
+              %ArgumentError{
+                message:
+                  ":sync_threshold and :discard_threshold cannot be used together, one of them must be nil"
+              },
+              _}}} =
+             :logger.update_handler_config(
+               @handler_name,
+               :config,
+               Map.put(config, :discard_threshold, 1)
+             )
+  end
+
+  defp register_delay do
+    bypass = Bypass.open()
+
+    put_test_config(
+      dsn: "http://public:secret@localhost:#{bypass.port}/1",
+      dedup_events: false,
+      hackney_opts: [recv_timeout: 500, pool: :sentry_pool]
+    )
+
+    Bypass.expect(bypass, fn conn ->
+      assert conn.request_path == "/api/1/envelope/"
+      assert conn.method == "POST"
+      Process.sleep(150)
+      Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>)
+    end)
+  end
+
+  defp register_before_send(context) do
     pid = self()
     ref = make_ref()
 
     put_test_config(
       before_send: fn event ->
         send(pid, {ref, event})
-        false
+
+        if Map.get(context, :send_request, false) do
+          event
+        else
+          false
+        end
       end,
       dsn: "http://public:secret@localhost:9392/1"
     )