Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Alerting: Add support for configuring avatar URL for the Discord notifier #33355

Merged
merged 7 commits into from May 28, 2021
Merged

Alerting: Add support for configuring avatar URL for the Discord notifier #33355

merged 7 commits into from May 28, 2021

Conversation

ChipWolf
Copy link
Contributor

@ChipWolf ChipWolf commented Apr 25, 2021 •
edited

What this PR does / why we need it: Similar to the Slack alert options, this exposes an avatar URL field for Discord alerts.

@ChipWolf ChipWolf requested a review from a team as a code owner April 25, 2021 01:01
@ChipWolf ChipWolf requested review from wbrowne and marefr and removed request for a team April 25, 2021 01:01
@CLAassistant
Copy link

CLAassistant commented Apr 25, 2021 •
edited

CLA assistant check
All committers have signed the CLA.

@grafanabot grafanabot added area/backend pr/external This PR is from external contributor labels Apr 25, 2021
@ChipWolf ChipWolf force-pushed the master branch 2 times, most recently from 46c70a5 to 39b6e4a Compare April 25, 2021 21:52
@ChipWolf
Copy link
Contributor Author

ChipWolf commented Apr 26, 2021 •
edited

@marefr let me know if you'd prefer any other optional config to be exposed in this PR, happy to expand the scope if required.

@marefr
Copy link
Member

marefr commented May 20, 2021

@ChipWolf thanks. We should be able to accept this. Just have to check a few things internally before coming back to reviewing this one. Until then there's some missing documentation for the Discord notifier if you're interested:

@marefr
Copy link
Member

marefr commented May 20, 2021

@ChipWolf we're currently in a migration phase why there are two instances of the discord notifier. May I ask you to sync with main branch and implement your changes in this file as well? https://github.com/grafana/grafana/blob/main/pkg/services/ngalert/notifier/channels/discord.go

Mentioned documentation would be a really good thing to have.

marefr
marefr requested changes May 20, 2021
View reviewed changes
Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See earlier comments

@ChipWolf
Copy link
Contributor Author

Will do @marefr
Also see #33377

@ChipWolf ChipWolf force-pushed the master branch 2 times, most recently from e19b348 to df20883 Compare May 20, 2021 21:03
@ChipWolf ChipWolf requested a review from a team as a code owner May 20, 2021 21:13
@ChipWolf ChipWolf requested a review from a team as a code owner May 20, 2021 21:29
@ChipWolf ChipWolf requested review from oddlittlebird, achatterjee-grafana, osg-grafana and marefr and removed request for a team May 20, 2021 21:29
achatterjee-grafana
achatterjee-grafana suggested changes May 20, 2021
View reviewed changes
Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some copy edit suggestions.

docs/sources/alerting/notifications.md Outdated Show resolved Hide resolved
docs/sources/alerting/notifications.md Outdated Show resolved Hide resolved
docs/sources/alerting/notifications.md Outdated Show resolved Hide resolved
@ChipWolf
Copy link
Contributor Author

ChipWolf commented May 20, 2021 •
edited

Thanks for reminding me my English scholarship from childhood aged like milk @achatterjee-grafana 😄

@achatterjee-grafana
Copy link
Contributor

Thanks for reminding me my English scholarship from my childhood aged like milk @achatterjee-grafana 😄

You were concentrating on the technical details, no worries :)

@achatterjee-grafana
Copy link
Contributor

This PR needs a milestone.

achatterjee-grafana
achatterjee-grafana approved these changes May 20, 2021
View reviewed changes
Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ChipWolf
Copy link
Contributor Author

@codesome thanks, let me know if there's anything else

ChipWolf and others added 7 commits May 25, 2021 22:29
@ChipWolf
Alerting: Add optional Discord configuration for avatar url
24c6629
@ChipWolf
New alerting: Add optional Discord configuration for avatar url
b6d8606
@ChipWolf
Docs: Add Discord notifications (closes grafana#34000)
35bd6d4
@ChipWolf
Ngalert: add available channel field for discord avatar url
eb1fbeb
@ChipWolf @achatterjee-grafana
Update docs/sources/alerting/notifications.md
acac757 
Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
@ChipWolf @achatterjee-grafana
Update docs/sources/alerting/notifications.md
c9e0c13 
Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
@ChipWolf @achatterjee-grafana
Update docs/sources/alerting/notifications.md
a6790ff 
Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
@ChipWolf ChipWolf requested a review from codesome May 25, 2021 21:29
@marefr marefr added this to the 8.0.0-beta3 milestone May 25, 2021
marefr
marefr approved these changes May 25, 2021
View reviewed changes
Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@marefr marefr changed the title Alerting: Add optional Discord configuration for avatar url Alerting: Add optional avatar URL configuration for the Discord notifier May 25, 2021
@marefr marefr merged commit badec6c into grafana:main May 28, 2021
@marefr marefr added the old backport v8.0.x Mark PR for automatic backport to v8.0.x label May 28, 2021
grafanabot pushed a commit that referenced this pull request May 28, 2021
@ChipWolf @grafanabot
Alerting: Add support for configuring avatar URL for the Discord noti…
e66f769 
…fier (#33355)

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
(cherry picked from commit badec6c)
@grafanabot grafanabot mentioned this pull request May 28, 2021
Merged
@marefr
Copy link
Member

marefr commented May 28, 2021

Thank you for contributing to Grafana!

Suggested for this to be backported to v8.0.x branch so that it will be included in v8.0. In normal cases I would have waited with this until 8.1, but given there's a lot of changes happening around alerting for v8.0 I thought v8.0.x is more appropriate.

@marefr marefr changed the title Alerting: Add optional avatar URL configuration for the Discord notifier Alerting: Add support for configuring avatar URL for the Discord notifier May 28, 2021
oddlittlebird pushed a commit that referenced this pull request May 31, 2021
@grafanabot @ChipWolf
Alerting: Add support for configuring avatar URL for the Discord noti…
2335c3d 
…fier (#33355) (#34940)

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
(cherry picked from commit badec6c)

Co-authored-by: Chip Wolf ‮ <hello@chipwolf.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marefr marefr marefr approved these changes

@codesome codesome codesome approved these changes

@achatterjee-grafana achatterjee-grafana achatterjee-grafana approved these changes

@wbrowne wbrowne Awaiting requested review from wbrowne

@oddlittlebird oddlittlebird Awaiting requested review from oddlittlebird

@osg-grafana osg-grafana Awaiting requested review from osg-grafana

Assignees
No one assigned
Labels
add to changelog area/backend old backport v8.0.x Mark PR for automatic backport to v8.0.x pr/external This PR is from external contributor type/docs type/feature-request
Projects
None yet
Milestone
8.0.0-beta3
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@ChipWolf @CLAassistant @marefr @achatterjee-grafana @codesome @grafanabot