Alerting: Add support for Sensu Go notification channel #28012

nixwiz · 2020-10-02T19:26:28Z

What this PR does / why we need it:
Similar to current support for the older "Core" version of
Sensu, this commit add support for the newer version.

Which issue(s) this PR fixes:

Fixes #19908

Special notes for your reviewer:

Similar to current support for the older "Core" version of Sensu, this commit add support for the newer version. Closes grafana#19908 Signed-off-by: Todd Campbell <todd@sensu.io>

Signed-off-by: Todd Campbell <todd@sensu.io>

hugohaggmark

Frontend changes look ok a bit confusing with 2 versions of sensu but I hope there is a description somewhere that helps the users?

aknuds1 · 2020-10-05T10:04:47Z

Thanks, we'll review it this week.

mckn

I agree with what @hugohaggmark said. What is the difference? Can it be expressed in a better way?

public/app/types/alerting.ts

aknuds1

I saw some issues that should be fixed, but bear in mind that we need to decide first if we want to include this new channel.

pkg/services/alerting/notifiers/sensugo.go

pkg/services/alerting/notifiers/sensugo_test.go

PR review suggestions Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

Signed-off-by: Todd Campbell <todd@sensu.io>

aknuds1 · 2020-10-12T16:04:50Z

We're still discussing potential inclusion of Sensu Go support.

achatterjee-grafana

Added two copy-edit suggestions.

pkg/services/alerting/notifiers/sensugo.go

Signed-off-by: Todd Campbell <todd@sensu.io>

aknuds1

We have thought about this a bit, and are leaning towards accepting the PR.

However, we need facilities to make it maintainable. Could you also add a Sensu Go block to devenv/docker/blocks, so we can easily bring up a Docker Compose environment when we need to maintain/test it?

Signed-off-by: Todd Campbell <todd@sensu.io>

nixwiz · 2020-10-22T19:07:29Z

There seems to be something wrong when wrong it with our devenv:

This should be fixed now.

devenv/docker/blocks/sensugo/docker-compose.yaml

Signed-off-by: Todd Campbell <todd@sensu.io>

kylebrandt

Works with devenv for me

papagian

Generally works fine!
I have requested some changes regarding securely storing the API key.

devenv/docker/blocks/sensugo/notes.md

pkg/services/alerting/notifiers/sensugo.go

Set the API key as a secure value. Co-authored-by: Sofia Papagiannaki <papagian@users.noreply.github.com>

…su-go

Signed-off-by: Todd Campbell <todd@sensu.io>

papagian

Thank you for your contribution and the delayed response.
It works fine but I have a small suggestion.

pkg/services/alerting/notifiers/sensugo.go

Co-authored-by: Sofia Papagiannaki <papagian@users.noreply.github.com>

papagian

Great! one last thing: you have to synchronise your branch with master once again for the CI built to succeed in order to get it merged!

nixwiz · 2020-11-26T15:47:25Z

@papagian branch merged with master and pushed again, CI builds successful. Thanks for your assistance!

aknuds1

Generally looks good to me, but left suggestions for some improvements to the tests. Basically, you should use require.NoError and require.Error instead of their assert equivalents, since you want to abort the tests if these tests fail, to avoid chaos/undefined behaviour in the following code.

pkg/services/alerting/notifiers/sensugo_test.go

Signed-off-by: Todd Campbell <todd@sensu.io>

Add support for Sensu Go notification channel

c058c1f

Similar to current support for the older "Core" version of Sensu, this commit add support for the newer version. Closes grafana#19908 Signed-off-by: Todd Campbell <todd@sensu.io>

nixwiz requested review from achatterjee-grafana, oddlittlebird and a team as code owners October 2, 2020 19:26

nixwiz requested review from a team, papagian, kylebrandt, mckn and hugohaggmark and removed request for a team October 2, 2020 19:26

fix linter errors

3b27c84

Signed-off-by: Todd Campbell <todd@sensu.io>

hugohaggmark reviewed Oct 5, 2020

View reviewed changes

aknuds1 added area/alerting/notifications Issues when sending alert notifications area/backend pr/external This PR is from external contributor type/feature labels Oct 5, 2020

mckn reviewed Oct 5, 2020

View reviewed changes

public/app/types/alerting.ts Show resolved Hide resolved

aknuds1 added the pr/needs-discussion This PR needs a bigger discussion label Oct 6, 2020

aknuds1 requested changes Oct 7, 2020

View reviewed changes

Todd Campbell and others added 3 commits October 7, 2020 06:36

Apply suggestions from code review

bb5bf28

PR review suggestions Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

Fix no new variables error

73f5a05

Replace convey testing with testify

75b8e1e

Signed-off-by: Todd Campbell <todd@sensu.io>

aknuds1 self-requested a review October 8, 2020 06:36

achatterjee-grafana reviewed Oct 13, 2020

View reviewed changes

pkg/services/alerting/notifiers/sensugo.go Outdated Show resolved Hide resolved

pkg/services/alerting/notifiers/sensugo.go Outdated Show resolved Hide resolved

Wording suggestions

6c12d78

Signed-off-by: Todd Campbell <todd@sensu.io>

aknuds1 reviewed Oct 14, 2020

View reviewed changes

Todd Campbell added 2 commits October 22, 2020 11:11

Merge remote-tracking branch 'upstream/master' into add-sensu-go

f69d23f

Renamed and fixed docker-compose.yaml to work in devenv

9d2f165

Signed-off-by: Todd Campbell <todd@sensu.io>

kylebrandt reviewed Oct 23, 2020

View reviewed changes

devenv/docker/blocks/sensugo/docker-compose.yaml Outdated Show resolved Hide resolved

Change sensugo web UI port to 3080 so as not to conflict with grafana

54f886e

Signed-off-by: Todd Campbell <todd@sensu.io>

kylebrandt approved these changes Oct 23, 2020

View reviewed changes

nixwiz requested a review from aknuds1 October 27, 2020 12:56

jessabe added type/feature-request and removed type/feature labels Nov 11, 2020

papagian requested changes Nov 18, 2020

View reviewed changes

devenv/docker/blocks/sensugo/notes.md Show resolved Hide resolved

pkg/services/alerting/notifiers/sensugo.go Show resolved Hide resolved

pkg/services/alerting/notifiers/sensugo.go Outdated Show resolved Hide resolved

Todd Campbell and others added 2 commits November 18, 2020 11:55

Merge remote-tracking branch 'upstream/master' into add-sensu-go

0ee2235

Apply suggestions from code review

76d2322

Set the API key as a secure value. Co-authored-by: Sofia Papagiannaki <papagian@users.noreply.github.com>

nixwiz requested a review from a team as a code owner November 18, 2020 17:02

Todd Campbell added 2 commits November 18, 2020 12:04

Merge branch 'add-sensu-go' of github.com:nixwiz/grafana into add-sen…

ec1e674

…su-go

Add Sensu Go information to notifications doc

7ec9271

Signed-off-by: Todd Campbell <todd@sensu.io>

nixwiz requested review from papagian and removed request for a team November 18, 2020 17:30

papagian requested changes Nov 26, 2020

View reviewed changes

pkg/services/alerting/notifiers/sensugo.go Outdated Show resolved Hide resolved

Update pkg/services/alerting/notifiers/sensugo.go

8675852

Co-authored-by: Sofia Papagiannaki <papagian@users.noreply.github.com>

papagian approved these changes Nov 26, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into add-sensu-go

47ae972

aknuds1 approved these changes Nov 26, 2020

View reviewed changes

change assert to require for Error/NoError tests

0ba3d32

Signed-off-by: Todd Campbell <todd@sensu.io>

papagian added this to the 7.4.0 milestone Nov 27, 2020

papagian merged commit 643f790 into grafana:master Nov 27, 2020

aknuds1 added the add to changelog label Nov 27, 2020

nixwiz deleted the add-sensu-go branch November 30, 2020 12:41

mjseaman added this to the 7.4.0 milestone Jan 8, 2021

calebhailey mentioned this pull request Jan 12, 2021

Allow silencing based on event labels sensu/sensu-go#4146

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alerting: Add support for Sensu Go notification channel #28012

Alerting: Add support for Sensu Go notification channel #28012

nixwiz commented Oct 2, 2020 •

edited

Loading

hugohaggmark left a comment

aknuds1 commented Oct 5, 2020

mckn left a comment

aknuds1 left a comment

aknuds1 commented Oct 12, 2020

achatterjee-grafana left a comment

aknuds1 left a comment

nixwiz commented Oct 22, 2020

kylebrandt left a comment

papagian left a comment

papagian left a comment

papagian left a comment

nixwiz commented Nov 26, 2020

aknuds1 left a comment •

edited

Loading

Alerting: Add support for Sensu Go notification channel #28012

Alerting: Add support for Sensu Go notification channel #28012

Conversation

nixwiz commented Oct 2, 2020 • edited Loading

hugohaggmark left a comment

Choose a reason for hiding this comment

aknuds1 commented Oct 5, 2020

mckn left a comment

Choose a reason for hiding this comment

aknuds1 left a comment

Choose a reason for hiding this comment

aknuds1 commented Oct 12, 2020

achatterjee-grafana left a comment

Choose a reason for hiding this comment

aknuds1 left a comment

Choose a reason for hiding this comment

nixwiz commented Oct 22, 2020

kylebrandt left a comment

Choose a reason for hiding this comment

papagian left a comment

Choose a reason for hiding this comment

papagian left a comment

Choose a reason for hiding this comment

papagian left a comment

Choose a reason for hiding this comment

nixwiz commented Nov 26, 2020

aknuds1 left a comment • edited Loading

Choose a reason for hiding this comment

nixwiz commented Oct 2, 2020 •

edited

Loading

aknuds1 left a comment •

edited

Loading