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 26 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 timeout

If you've defined a triggering strategy which holds off notification delivering you can define a timeout value
which will be used to deliver the notification after timeout milliseconds have passed from the last error. The
timeout 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],
throttle_timeout: :timer.minutes(30),
notifier: CustomNotifier

# ...
end
```


## Custom data or Metadata

By default, `BoomNotifier` will **not** include any custom data from your
Expand Down
86 changes: 5 additions & 81 deletions lib/boom_notifier.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,55 +3,10 @@ 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])

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

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

def walkthrough_notifiers(settings, callback) do
case Keyword.get(settings, :notifiers) do
nil ->
run_callback(settings, callback)

notifiers_settings when is_list(notifiers_settings) ->
Enum.each(notifiers_settings, &run_callback(&1, callback))
end
end

def validate_notifiers(notifier, options) do
if Code.ensure_loaded?(notifier) && function_exported?(notifier, :validate_config, 1) do
case notifier.validate_config(options) do
{:error, message} ->
Logger.error(
"Notifier validation: #{message} in #{notifier |> to_string() |> String.split(".") |> List.last()}"
)

_ ->
nil
end
end
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 +23,13 @@ defmodule BoomNotifier do
end

# Notifiers validation
walkthrough_notifiers(
unquote(config),
fn notifier, options -> validate_notifiers(notifier, options) end
BoomNotifier.Api.walkthrough_notifiers(
config,
fn notifier, options -> BoomNotifier.Api.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.Api.notify_error(unquote(config), conn, error)
end
end
end
Expand Down
70 changes: 70 additions & 0 deletions lib/boom_notifier/api.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
defmodule BoomNotifier.Api do
@moduledoc """
BoomNotifier Entrypoint API

Define functions required by BoomNotifier module when used on a plug app.
"""
require Logger

alias BoomNotifier.ErrorInfo
alias BoomNotifier.NotificationSender

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

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 ->
run_callback(settings, callback)

notifiers_settings when is_list(notifiers_settings) ->
Enum.each(notifiers_settings, &run_callback(&1, callback))
end
end

def validate_notifiers(notifier, options) do
if Code.ensure_loaded?(notifier) && function_exported?(notifier, :validate_config, 1) do
case notifier.validate_config(options) do
{:error, message} ->
Logger.error(
"Notifier validation: #{message} in #{notifier |> to_string() |> String.split(".") |> List.last()}"
)

_ ->
nil
end
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
end
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
27 changes: 27 additions & 0 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 Down Expand Up @@ -50,6 +51,7 @@ defmodule BoomNotifier.ErrorInfo do
name: error_name,
metadata: build_custom_data(conn, custom_data_strategy)
}
|> ensure_key()
germanbottosur marked this conversation as resolved.
Show resolved Hide resolved
end

defp error_reason(%name{message: reason}), do: {reason, name}
Expand Down Expand Up @@ -116,4 +118,29 @@ 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
@spec generate_error_key(__MODULE__.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

def ensure_key(%{key: nil} = error_info) do
error_info |> Map.put(:key, generate_error_key(error_info))
end

def ensure_key(%{} = error_info) do
error_info |> Map.put_new_lazy(:key, fn -> generate_error_key(error_info) end)
end
germanbottosur marked this conversation as resolved.
Show resolved Hide resolved
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} = ErrorInfo.ensure_key(error_info)
iaguirre88 marked this conversation as resolved.
Show resolved Hide resolved
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} = ErrorInfo.ensure_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} = ErrorInfo.ensure_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} = ErrorInfo.ensure_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} = ErrorInfo.ensure_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} = ErrorInfo.ensure_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