feat(notifier): add ability to scale webhook and script #961
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #961 +/- ##
==========================================
+ Coverage 68.20% 68.31% +0.10%
==========================================
Files 208 208
Lines 11731 11768 +37
==========================================
+ Hits 8001 8039 +38
Misses 3251 3251
+ Partials 479 478 -1 ☔ View full report in Codecov by Sentry. |
|
/build |
|
Build and push Docker images with tag: feat-add-ability-to-scale-senders.2023-11-28.991ac7d |
2 similar comments
|
Build and push Docker images with tag: feat-add-ability-to-scale-senders.2023-11-28.991ac7d |
|
Build and push Docker images with tag: feat-add-ability-to-scale-senders.2023-11-28.991ac7d |
notifier/notifier.go
Outdated
| metrics *metrics.NotifierMetrics | ||
| metricSourceProvider *metricSource.SourceProvider | ||
| imageStores map[string]moira.ImageStore | ||
| sendersNameToType map[string]string |
There was a problem hiding this comment.
это для меня пометочка: смотрю зачем он остался...
There was a problem hiding this comment.
это тоже мне, если нужно, то зачем такой тип поля, кажется, можно иначе
и перенести ближе к сендерам - по смыслу, чтоб не разносить
There was a problem hiding this comment.
Кажется, по смыслу оно не зависит от каждого конкретного сендера, а нужно именно для нотифаера. Поле маппит имя типа контакта = имени сендера на тип сендера, а если поле с именем не указано, как сейчас для почти всех сендеров, то маппит тип контакта = типу сендера на тип сендера
Таким образом, если хочется несколько сендеров одного типа, то достаточно добавить поле name и смотреть, чтобы оно совпадало с типом контакта
There was a problem hiding this comment.
вот в этом Issue пользователь как раз написал, что такой вариант нужен и я как раз такой и сделал
| if sender.url == moira.VariableContactValue { | ||
| sender.log.Warning(). | ||
| String("potentially_dangerous_url", sender.url). | ||
| func (client *webhookClient) buildRequest(events moira.NotificationEvents, contact moira.ContactData, trigger moira.TriggerData, plots [][]byte, throttled bool) (*http.Request, error) { |
There was a problem hiding this comment.
не поняла, почему сендер поменялся на вебхук-клиент...
There was a problem hiding this comment.
У нас есть 1 структура для вебхука, если мы хотим иметь несколько хуков, то я внутри этой структуры сендера сделал мапку, внутри которой будут поддерживаться несколько клиентов, а тип для функции сменился, потому что в каждом клиенте своя специфичная информация, которая нужна для построения запроса
| @@ -21,10 +21,107 @@ import ( | |||
| const ( | |||
| testUser = "testUser" | |||
There was a problem hiding this comment.
давай тут сразу сделаем идентично
а то и верблюды и змеи :)
There was a problem hiding this comment.
Давай я в целом уберу лишние изменения в файлах, которые на будущее были сделаны. Чтобы проще было ориентироваться
notifier/notifier.go
Outdated
| } | ||
| } | ||
|
|
||
| // Send is realization of StandardNotifier Send functionality | ||
| func (notifier *StandardNotifier) Send(pkg *NotificationPackage, waitGroup *sync.WaitGroup) { | ||
| ch, found := notifier.senders[pkg.Contact.Type] | ||
| if !found { | ||
| senderType, ok := notifier.sendersNameToType[pkg.Contact.Type] |
There was a problem hiding this comment.
pkg.Contact.Type - это на самом деле уникальный идентификатор?
There was a problem hiding this comment.
не поняла, как мы ловко определим тут, что это нужная нам почта/вебхук/и тд
There was a problem hiding this comment.
pkg.Contact.Type - у всех же будет лежать как лежал старое значение
There was a problem hiding this comment.
Да, будет лежать старое значение и все будет норм работать, тк будет матчиться тип сендера на тип контакта, а если пользователи захотят добавить несколько сендеров одного типа, то необходимо различать по полю name, которое должно будет совпадать с типом контакта
There was a problem hiding this comment.
кажется, что-то непрозрачное
у меня была одна почта
потом появилась вторая
я первую назову: Старая почта, а новую - Новая почта
| @@ -126,7 +143,7 @@ func (notifier *StandardNotifier) Send(pkg *NotificationPackage, waitGroup *sync | |||
| // GetSenders get hash of registered notifier senders | |||
| func (notifier *StandardNotifier) GetSenders() map[string]bool { | |||
Add ability to scale webhook and script
For scaling webhooks and scripts, I used the
namefield, by which I distinguish between different senders of the same type. In addition, I fixed a problem with metrics registry when adding several senders of the same typeIMPORTANT, the
namefield in the senders config must match thetypefield in the api config if you want multiple senders of the same type, if you don't need to add multiple senders of the same type, thenamefield is optionalRelates #821