Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to discard events under load #849

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions lib/sentry/logger_handler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ 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
Expand All @@ -86,6 +86,22 @@ defmodule Sentry.LoggerHandler do
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**.

`:sync_threshold` and `:discard_threshold` cannot be used together. To disable this option,
set it to `nil`.
"""
],
discard_threshold: [
type: {:or, [nil, :non_neg_integer]},
default: nil,
doc: """
The number of queued events after which this handler will start
to **discard** events. This option effectively implements **load shedding**.

*Available since v10.8.2*.

`:discard_threshold` and `:sync_threshold` cannot be used together. To disable this option,
set it to `nil`.
"""
]
]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -319,6 +336,10 @@ defmodule Sentry.LoggerHandler do
config.rate_limiting && RateLimiter.increment(handler_id) == :rate_limited ->
:ok

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).
Expand Down Expand Up @@ -397,7 +418,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(
Expand Down Expand Up @@ -632,7 +660,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
Expand Down
72 changes: 70 additions & 2 deletions test/sentry/logger_handler_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@
end

@tag handler_config: %{capture_log_messages: true}
test "works with changing config to enable rate limiting", %{sender_ref: ref} do

Check failure on line 535 in test/sentry/logger_handler_test.exs

View workflow job for this annotation

GitHub Actions / Test (Elixir 1.18, OTP 27.2)

test rate limiting works with changing config to enable rate limiting (Sentry.LoggerHandlerTest)
for index <- 1..10 do
message = "Message #{index}"
Logger.error(message)
Expand Down Expand Up @@ -601,14 +601,82 @@
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"
)
Expand Down
Loading