Split the notify package into sub packages #1929
Conversation
Instead of keeping all notifiers in the notify package, it splits them into individual sub-packages. This improves readability and maintainability of the code. Signed-off-by: Simon Pasquier <[email protected]>
| @@ -92,6 +101,54 @@ func instrumentHandler(handlerName string, handler http.HandlerFunc) http.Handle | |||
|
|
|||
| const defaultClusterAddr = "0.0.0.0:9094" | |||
|
|
|||
| // buildReceiverIntegrations builds a list of integration notifiers off of a | |||
| // receiver config. | |||
| func buildReceiverIntegrations(nc *config.Receiver, tmpl *template.Template, logger log.Logger) ([]notify.Integration, error) { | |||
There was a problem hiding this comment.
Shouldn't this be inside the github.com/prometheus/alertmanager/notify package?
There was a problem hiding this comment.
Depending on @simonpasquier's reasoning, I could understand it being moved here. It seems like this is a bunch of imperative code using the notify package components, so it's kind of the glue tying together a library? 🤷♂️, will wait to see what he has to say.
There was a problem hiding this comment.
I saw no concrete reason to keep it in github.com/prometheus/alertmanager/notify as it is the glue between configuration and notifiers.
Signed-off-by: Simon Pasquier <[email protected]>
simonpasquier
force-pushed
the
refactor-notify
branch
from
38d383d to
5f881d6
Compare
| command: make | ||
| environment: | ||
| # Run garbage collection more aggresively to avoid getting OOMed. | ||
| GOGC: "20" |
There was a problem hiding this comment.
Am I right that CircleCI gives us an 8gb machine? https://circleci.com/docs/2.0/executor-types/
I am surprised that we are being OOMed in that case.
There was a problem hiding this comment.
No we got 4GB because we use the Docker executor with the free plan. We could use a machine executor but it takes more time to start. IMO we need to ask the CNCF for a paid account that would allow to use bigger resources. I can bring it up during our next sync meeting.
There was a problem hiding this comment.
I'm surprised we even hit 4gb, to be honest.
Instead of keeping all notifiers in the notify package, it splits them into individual sub-packages. This improves readability and maintainability of the code.
Relates to #1400 and #1517.