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.

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

Alerting: Ensuring LINE Notify notifications are sent for all alert states #27639

Merged
merged 3 commits into from
Sep 24, 2020

Conversation

haraldkubota
Copy link
Contributor

@haraldkubota haraldkubota commented Sep 17, 2020

What this PR does / why we need it:

LINE Notifications only work for alerts. The "OK" notifications does not get sent. Other notifiers (email, Slack) do send both "[Alerting]" as well as "[OK]" notifications.

Which issue(s) this PR fixes:

Fixes LINE Notifications now OK notifications too. Also the notification contains the "[Alerting]" resp. "[OK]" like email and Slack do.

Fixes specific issue #12781

Special notes for your reviewer:

I compared other notifiers and none seem to have the problem of missing "OK" notifications. I guess LINE is not used a lot.

@haraldkubota haraldkubota requested a review from a team as a code owner September 17, 2020 15:52
@haraldkubota haraldkubota requested review from papagian and marefr and removed request for a team September 17, 2020 15:52
@CLAassistant
Copy link

CLAassistant commented Sep 17, 2020

CLA assistant check
All committers have signed the CLA.

@marefr
Copy link
Contributor

marefr commented Sep 17, 2020

Thanks. Please sign the Contributor License Agreement (CLA). We'll not review this until it's signed.

@marefr marefr added area/alerting/notifications Issues when sending alert notifications pr/external This PR is from external contributor area/backend pr/cla-not-signed labels Sep 17, 2020
@marefr marefr requested review from wbrowne and removed request for papagian September 18, 2020 08:50
Copy link
Member

@wbrowne wbrowne left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @haraldkubota! 😄 Looks good!

@wbrowne wbrowne added this to the 7.3 milestone Sep 24, 2020
@wbrowne wbrowne merged commit 81bf9fc into grafana:master Sep 24, 2020
@wbrowne wbrowne changed the title LINE Notify to have Alerts and OK notifications Alerting: Ensuring LINE Notify notifications are sent for all alert states Sep 24, 2020
@wbrowne wbrowne mentioned this pull request Oct 15, 2020
ryantxu pushed a commit that referenced this pull request Nov 18, 2020
* Get Alerts AND OKs for LINE Notify

* Get Alerts AND OKs for LINE Notify

* Get Alerts AND OKs for LINE Notify
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/alerting/notifications Issues when sending alert notifications area/backend pr/external This PR is from external contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants