-
Notifications
You must be signed in to change notification settings - Fork 13
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
Refactor error storage data #79
Conversation
e71e5be
to
97d0294
Compare
Currently, we are grouping every error information in several lists according to its error type. Eventually, this could consume a lot of memory without providing any important benefit (the only piece of information that changes is the timestamp and the request data). This commit replaces that with a map structure where its key is the hashed error info and its value is the actual error info plus: * the number of times this error occurred, * the first and the list time it happened.
97d0294
to
da24a16
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 🚀 🚀
Really like how this ended!!! Also like the use of the first entry of the stacktrace for the hash key (but not entirely).
I think we need to update the readme in the Custom Notifiers
section, right? I was also thinking that maybe it's worth to let the user know in that part of the readme something of what that ErrorInfo
struct could have? (I know it's in the code, but just for attract more 😛)
lib/boom_notifier/error_storage.ex
Outdated
accumulated_occurrences = Map.get(error_storage_item, :accumulated_occurrences) | ||
max_storage_capacity = Map.get(error_storage_item, :__max_storage_capacity__) | ||
|
||
if accumulated_occurrences >= max_storage_capacity, do: true, else: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: is this if
necessary? 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not at all 😅
This is a breaking change
Currently, we are grouping every error information in several lists
according to its error type. Eventually, this could consume a lot of
memory without providing any important benefit (the only piece of
information that changes is the timestamp and the request data).
This commit replaces that with a map structure where its key is the
hashed error info and its value is the actual error info plus:
Email example:
Webhook example: