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

[Feature] Notification delivery throttling timeout #92

Merged
merged 32 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
1ecaa31
refactor code location
joaquinco Aug 8, 2024
35f7fb1
use test module as pid name for tests
joaquinco Aug 9, 2024
b4b9202
refactor genserver logic
joaquinco Aug 9, 2024
666b05e
fix notifier tests
joaquinco Aug 9, 2024
9e725fb
dont send messages back in tests if pid is nil
joaquinco Aug 12, 2024
483690c
add grouping timeout implementation
joaquinco Aug 12, 2024
b1674e5
refactor genserver name
joaquinco Aug 12, 2024
29921ac
compute and save error info key
joaquinco Aug 12, 2024
828be3a
add notification sender tests
joaquinco Aug 13, 2024
62b945f
fix in error info build
joaquinco Aug 13, 2024
6ba8010
test error storage accumulated values
joaquinco Aug 13, 2024
63e178b
update README
joaquinco Aug 13, 2024
0ef3bd3
fix error info key generation
joaquinco Aug 13, 2024
1a1a91b
mailer tests: flush mesages on_exit
joaquinco Aug 14, 2024
974005c
notification sender tests: cancel timers on_exit
joaquinco Aug 14, 2024
6319a99
run tests that change global state sync
joaquinco Aug 14, 2024
eff0132
add test util to clear error storage
joaquinco Aug 14, 2024
b7d87b8
clear timers between test cases
joaquinco Aug 14, 2024
cf930a5
add module doc
joaquinco Aug 22, 2024
f3f32b7
fixes from credo checks
joaquinco Aug 22, 2024
fc968f4
undo change to example_app test
joaquinco Aug 23, 2024
5eac749
use TestMessageProxy for assert receive tests
joaquinco Aug 23, 2024
b3fd0af
fix accumulated_occurrences reset before notify
joaquinco Aug 23, 2024
9047686
don't send notifier messages on error_info_test
joaquinco Aug 26, 2024
98ff12e
Remove walkthrough_notifiers from use BoomNotifier
joaquinco Aug 29, 2024
58c5b21
fix 404 /favicon.ico
joaquinco Sep 2, 2024
ef8512e
changes from review
joaquinco Sep 4, 2024
71c0a22
unify fake mailers
joaquinco Sep 5, 2024
d34bd19
add dummy mailer
joaquinco Sep 12, 2024
c6f30a5
remove api module
joaquinco Sep 17, 2024
b8a352d
generate error key in build only
joaquinco Sep 17, 2024
bad34aa
change throttle_timeout to time_limit
joaquinco Sep 17, 2024
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
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ erl_crash.dump
boom-*.tar

.DS_Store
priv/
/priv/
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,26 @@ defmodule YourApp.Router do
]
```

### Notification trigger time limit

If you've defined a triggering strategy which holds off notification delivering you can define a time limit value
which will be used to deliver the notification after a time limit milliseconds have passed from the last error. The
time counter is reset on new errors and only applies for cases where notifications are not sent.

```elixir
defmodule YourApp.Router do
use Phoenix.Router

use BoomNotifier,
notification_trigger: [:exponential],
time_limit: :timer.minutes(30),
notifier: CustomNotifier

# ...
end
```


## Custom data or Metadata

By default, `BoomNotifier` will **not** include any custom data from your
Expand Down
90 changes: 38 additions & 52 deletions lib/boom_notifier.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,22 @@ defmodule BoomNotifier do

# Responsible for sending a notification to each notifier every time an
# exception is raised.

alias BoomNotifier.ErrorInfo
alias BoomNotifier.ErrorStorage
alias BoomNotifier.NotifierSenderServer
require Logger

def run_callback(settings, callback) do
missing_keys = Enum.reject([:notifier, :options], &Keyword.has_key?(settings, &1))

case missing_keys do
[] ->
callback.(settings[:notifier], settings[:options])
alias BoomNotifier.ErrorInfo
alias BoomNotifier.NotificationSender

[missing_key] ->
Logger.error("(BoomNotifier) #{inspect(missing_key)} parameter is missing")
def notify_error(settings, conn, %{kind: :error, reason: %mod{}} = error) do
ignored_exceptions = Keyword.get(settings, :ignore_exceptions, [])

_ ->
Logger.error(
"(BoomNotifier) The following parameters are missing: #{inspect(missing_keys)}"
)
unless Enum.member?(ignored_exceptions, mod) do
trigger_notify_error(settings, conn, error)
end
end

def notify_error(settings, conn, error),
do: trigger_notify_error(settings, conn, error)

def walkthrough_notifiers(settings, callback) do
case Keyword.get(settings, :notifiers) do
nil ->
Expand All @@ -50,8 +43,32 @@ defmodule BoomNotifier do
end
end

defp run_callback(settings, callback) do
missing_keys = Enum.reject([:notifier, :options], &Keyword.has_key?(settings, &1))

case missing_keys do
[] ->
callback.(settings[:notifier], settings[:options])

[missing_key] ->
Logger.error("(BoomNotifier) #{inspect(missing_key)} parameter is missing")

_ ->
Logger.error(
"(BoomNotifier) The following parameters are missing: #{inspect(missing_keys)}"
)
end
end

defp trigger_notify_error(settings, conn, error) do
custom_data = Keyword.get(settings, :custom_data, :nothing)
error_info = ErrorInfo.build(error, conn, custom_data)

NotificationSender.async_trigger_notify(settings, error_info)
end

defmacro __using__(config) do
quote do
quote location: :keep, bind_quoted: [config: config] do
import BoomNotifier

error_handler_in_use = Plug.ErrorHandler in @behaviour
Expand All @@ -68,44 +85,13 @@ defmodule BoomNotifier do
end

# Notifiers validation
walkthrough_notifiers(
unquote(config),
fn notifier, options -> validate_notifiers(notifier, options) end
BoomNotifier.walkthrough_notifiers(
config,
fn notifier, options -> BoomNotifier.validate_notifiers(notifier, options) end
)

def notify_error(conn, %{kind: :error, reason: %mod{}} = error) do
settings = unquote(config)
{ignored_exceptions, _settings} = Keyword.pop(settings, :ignore_exceptions, [])

unless Enum.member?(ignored_exceptions, mod) do
do_notify_error(conn, settings, error)
end
end

def notify_error(conn, error) do
do_notify_error(conn, unquote(config), error)
end

defp do_notify_error(conn, settings, error) do
{custom_data, _settings} = Keyword.pop(settings, :custom_data, :nothing)
error_info = ErrorInfo.build(error, conn, custom_data)

ErrorStorage.store_error(error_info)

if ErrorStorage.send_notification?(error_info) do
notification_data =
Map.put(error_info, :occurrences, ErrorStorage.get_error_stats(error_info))

# Triggers the notification in each notifier
walkthrough_notifiers(settings, fn notifier, options ->
NotifierSenderServer.send(notifier, notification_data, options)
end)

{notification_trigger, _settings} =
Keyword.pop(settings, :notification_trigger, :always)

ErrorStorage.reset_accumulated_errors(notification_trigger, error_info)
end
BoomNotifier.notify_error(unquote(config), conn, error)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/boom_notifier/application.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ defmodule BoomNotifier.Application do
def start(_type, _args) do
children = [
BoomNotifier.ErrorStorage,
BoomNotifier.NotifierSenderServer
BoomNotifier.NotificationSender
]

Supervisor.start_link(children, strategy: :one_for_one)
Expand Down
23 changes: 21 additions & 2 deletions lib/boom_notifier/error_info.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ defmodule BoomNotifier.ErrorInfo do

@enforce_keys [:reason, :stack, :timestamp]
defstruct [
:key,
:name,
:reason,
:stack,
Expand All @@ -34,13 +35,13 @@ defmodule BoomNotifier.ErrorInfo do
required(:stack) => Exception.stacktrace(),
optional(any()) => any()
},
map(),
Plug.Conn.t(),
custom_data_strategy_type
) :: __MODULE__.t()
def build(%{reason: reason, stack: stack}, conn, custom_data_strategy) do
{error_reason, error_name} = error_reason(reason)

%__MODULE__{
error_info = %__MODULE__{
reason: error_reason,
stack: stack,
controller: get_in(conn.private, [:phoenix_controller]),
Expand All @@ -50,6 +51,8 @@ defmodule BoomNotifier.ErrorInfo do
name: error_name,
metadata: build_custom_data(conn, custom_data_strategy)
}

error_info |> Map.put(:key, generate_error_key(error_info))
end

defp error_reason(%name{message: reason}), do: {reason, name}
Expand Down Expand Up @@ -116,4 +119,20 @@ defmodule BoomNotifier.ErrorInfo do
Enum.reduce(options, %{}, fn opt, acc ->
Map.merge(acc, build_custom_data(conn, opt))
end)

# Generates a unique hash key based on the error info. The timestamp and the
# request info is removed so we don't get different keys for the same error.
#
# The map is converted to a string using `inspect()` so we can hash it using
# the crc32 algorithm that was taken from the Exception Notification library
# for Rails
defp generate_error_key(error_info) do
error_info
|> Map.delete(:request)
|> Map.delete(:metadata)
|> Map.delete(:timestamp)
|> Map.update(:stack, nil, fn stacktrace -> List.first(stacktrace) end)
|> inspect()
|> :erlang.crc32()
end
end
29 changes: 6 additions & 23 deletions lib/boom_notifier/error_storage.ex
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ defmodule BoomNotifier.ErrorStorage do
"""
@spec store_error(ErrorInfo.t()) :: :ok
def store_error(error_info) do
error_hash_key = generate_error_key(error_info)
%{key: error_hash_key} = error_info
timestamp = error_info.timestamp || DateTime.utc_now()

default_error_storage_info = %__MODULE__{
Expand Down Expand Up @@ -67,7 +67,7 @@ defmodule BoomNotifier.ErrorStorage do
"""
@spec get_error_stats(ErrorInfo.t()) :: %__MODULE__{}
def get_error_stats(error_info) do
error_hash_key = generate_error_key(error_info)
%{key: error_hash_key} = error_info

Agent.get(:boom_notifier, fn state -> state end)
|> Map.get(error_hash_key)
Expand All @@ -81,7 +81,7 @@ defmodule BoomNotifier.ErrorStorage do
"""
@spec send_notification?(ErrorInfo.t()) :: boolean()
def send_notification?(error_info) do
error_hash_key = generate_error_key(error_info)
%{key: error_hash_key} = error_info

error_storage_item =
Agent.get(:boom_notifier, fn state -> state end)
Expand All @@ -96,7 +96,7 @@ defmodule BoomNotifier.ErrorStorage do
"""
@spec reset_accumulated_errors(error_strategy, ErrorInfo.t()) :: :ok
def reset_accumulated_errors(:exponential, error_info) do
error_hash_key = generate_error_key(error_info)
%{key: error_hash_key} = error_info

Agent.update(
:boom_notifier,
Expand All @@ -108,7 +108,7 @@ defmodule BoomNotifier.ErrorStorage do
end

def reset_accumulated_errors([exponential: [limit: limit]], error_info) do
error_hash_key = generate_error_key(error_info)
%{key: error_hash_key} = error_info

Agent.update(
:boom_notifier,
Expand All @@ -120,7 +120,7 @@ defmodule BoomNotifier.ErrorStorage do
end

def reset_accumulated_errors(:always, error_info) do
error_hash_key = generate_error_key(error_info)
%{key: error_hash_key} = error_info

Agent.update(
:boom_notifier,
Expand All @@ -131,23 +131,6 @@ defmodule BoomNotifier.ErrorStorage do
)
end

# Generates a unique hash key based on the error info. The timestamp and the
# request info is removed so we don't get different keys for the same error.
#
# The map is converted to a string using `inspect()` so we can hash it using
# the crc32 algorithm that was taken from the Exception Notification library
# for Rails
@spec generate_error_key(ErrorInfo.t()) :: non_neg_integer()
def generate_error_key(error_info) do
error_info
|> Map.delete(:request)
|> Map.delete(:metadata)
|> Map.delete(:timestamp)
|> Map.update(:stack, nil, fn stacktrace -> List.first(stacktrace) end)
|> inspect()
|> :erlang.crc32()
end

defp clear_values(error_storage_item) do
error_storage_item
|> Map.replace!(:accumulated_occurrences, 0)
Expand Down
Loading
Loading