Skip to content

Commit

Permalink
[Feature] Notification delivery throttling timeout (#92)
Browse files Browse the repository at this point in the history
* refactor genserver logic

* add grouping time limit implementation

* refactor genserver name

* compute and save error info key

* clear timers between test cases

* fix 404 /favicon.ico
  • Loading branch information
joaquinco authored Sep 18, 2024
1 parent d22b680 commit 64f07d6
Show file tree
Hide file tree
Showing 19 changed files with 543 additions and 266 deletions.
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

0 comments on commit 64f07d6

Please sign in to comment.