-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Notifier: Webex #3132
Notifier: Webex #3132
Conversation
e7143a2
to
e176a65
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.
Looking good, a couple very small nits
While most integrations set a limit by UTF-8 compatible characters (some like Webex) use runes - as pointed out in #3132. This PR makes it explicit wether the truncation is happening at a byte or rune level.
While most integrations set a limit by UTF-8 compatible characters (some like Webex) use runes - as pointed out in #3132. This PR makes it explicit wether the truncation is happening at a byte or rune level. Signed-off-by: gotjosh <[email protected]>
Cisco's Webex has been one of the most requested notifiers on Grafana for a while now, please see: grafana/grafana#11750 (comment) Given it's straightforward implementation, low maintance overhead and request demand, I think it's worth including this directly in the Alertmanager. Signed-off-by: gotjosh <[email protected]>
…tup in Webex Teams - Move away from Webhook to APIRUL - Make the Room ID a require field - Set Authorization Credentails via Headers Signed-off-by: gotjosh <[email protected]>
Signed-off-by: gotjosh <[email protected]>
…present Signed-off-by: gotjosh <[email protected]>
…utchered Signed-off-by: gotjosh <[email protected]>
628312e
to
f51f51e
Compare
Signed-off-by: gotjosh <[email protected]>
While most integrations set a limit by UTF-8 compatible characters (some like Webex) use runes - as pointed out in prometheus#3132. This PR makes it explicit wether the truncation is happening at a byte or rune level. Signed-off-by: gotjosh <[email protected]> Signed-off-by: Yijie Qin <[email protected]>
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.
Looks good! I have a comment on the configuration + we would need an addition to https://github.com/prometheus/alertmanager/blob/main/docs/configuration.md#receiver.
While most integrations set a limit by UTF-8 compatible characters (some like Webex) use runes - as pointed out in prometheus#3132. This PR makes it explicit wether the truncation is happening at a byte or rune level. Signed-off-by: gotjosh <[email protected]> Signed-off-by: Yijie Qin <[email protected]>
Signed-off-by: gotjosh <[email protected]>
Signed-off-by: gotjosh <[email protected]>
Signed-off-by: gotjosh <[email protected]>
Signed-off-by: gotjosh <[email protected]>
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.
lgtm, albeit 2 minor comments on the doc part.
Signed-off-by: gotjosh <[email protected]>
Cisco's Webex has been one of the most requested notifiers on Grafana for a while now, please see: grafana/grafana#11750 (comment)
Given it's straightforward implementation, low maintenance overhead and request demand, I think it's worth including this directly in the Alertmanager.