opsgenie: Moved from deprecated, non documented teams to responders field. #1863

bwplotka · 2019-04-26T17:56:39Z

Fixes: #1818
Fixes: #1342

Some decisions I made:

"Teams" config option will fail on unmarshalling the config. I marked it as deprecated. AM is 0.x version it should be fine (?)
I allow the use case for dynamic set of responders from alert's label using templating. Seems like it fits into how others fields are treated.
I allow for full flexibility, so you can use any type you want. There is no validation. I don't think it is worth as Opsgenie changes those things (?). Let me know if it's needed. I think if the type is wrong and create alert will fail it's fine as "validation"

Signed-off-by: Bartek Plotka bwplotka@gmail.com

bwplotka · 2019-04-26T19:04:33Z

Will fix tests tomorrow.

bwplotka · 2019-04-27T20:10:31Z

Tests should be fixed.

simonpasquier

Thanks @bwplotka! I've left a few comments.

simonpasquier · 2019-04-29T08:37:44Z

.gitignore

@@ -9,7 +9,7 @@

 !.golangci.yml
 !/cli/testdata/*.yml
-!/cli/config/testdata/*.yml
+!/config/testdata/*.yml


!/cli/config/testdata/*.yml should be kept AFAICT.

simonpasquier · 2019-04-29T08:40:37Z

config/notifiers.go

+}
+
+type OpsGenieConfigResponder struct {
+	// One of those 3 should be filled.


Can we add a check for this when unmarshaling the struct and return an error if necessary?

simonpasquier · 2019-04-29T08:43:08Z

config/notifiers.go

+	}
+
+	if c.Teams != "" {
+		// Should we allow this for backward compatibility? Should we fill responders from this?


Given that we can't propagate the deprecation to the user in the logs, I think that a hard failure is preferred.

simonpasquier · 2019-04-29T08:48:31Z

config/notifiers.go

-	Tags        string            `yaml:"tags,omitempty" json:"tags,omitempty"`
-	Note        string            `yaml:"note,omitempty" json:"note,omitempty"`
-	Priority    string            `yaml:"priority,omitempty" json:"priority,omitempty"`
+	// Teams field is deprecated.


No strong feeling about this one but I'd rather remove the field as part of the change.

Up to you guys (: The problem is that we will not fail the config unmarshalling if people will be still on Teams (and they will be). Thoughts?

The unmarshaling should still fail as we use yaml.UnmarshalStrict().

bwplotka · 2019-04-30T20:41:28Z

Thanks for initial review @simonpasquier , PTAL

simonpasquier

LGTM. Delegating it to @mxinden and @stuartnelson3 now...

stuartnelson3

no complaints from me

simonpasquier · 2019-05-03T12:12:51Z

@bwplotka you would need to resolve the conflicts...

bwplotka · 2019-05-09T13:36:50Z

done (:

…ield. Teams config option will fail unmarshalling as it is deprecated. Fixes prometheus#1818 Signed-off-by: Bartek Plotka <bwplotka@gmail.com>

bwplotka · 2019-05-09T15:47:40Z

Is this build flake? After changelog rebase suddnely tests are red

simonpasquier · 2019-05-10T07:45:20Z

Yes, it looks like a flake, I've re-run the workflow.

hrancic · 2019-07-30T12:17:40Z

A question, if you would be so kind. Will the yml configuration file still work with alertmanager 0.18.0 if teams are set up according to https://prometheus.io/docs/alerting/configuration/#opsgenie_config ?

# Comma separated list of team responsible for notifications.
[ teams: <tmpl_string> ]

or should it be changed to something like:

responders:
     name: <tmpl_string>
     type: team

?

simonpasquier · 2019-07-30T13:14:45Z

@hrancic the documentation should be updated (and fixed) in a few minutes.

hrancic · 2019-08-02T11:04:18Z

Another question. Our current configurations look something like this:

teams: <tmpl_string>

these templates (which have some conditional logic) are creating a comma separated lists of teams. We would like to upgrade our alertmanager version. How do you propose we migrate to the new version?

bwplotka · 2019-08-02T11:13:00Z

@hrancic do you mean how to specify multiple teams?

responders:
- name: "teamA"
  type: team
- name: "teamB"
  type: team
- name: "teamC"
  type: team
(...)

hrancic · 2019-08-02T11:25:26Z

yes, but writing them one by one is out of the question. Will responders: <tmpl_string> work? Where this template creates the following from a list of teams:

- name: "teamA" 
  type: team
- name: "teamB"
  type: team
- name: "teamC"
  type: team
(...)

bwplotka · 2019-08-02T11:42:35Z

yes, but writing them one by one is out of the question

Why?

Will responders: <tmpl_string> work?

It will not work in current version. (:

hrancic · 2019-08-02T12:17:41Z

We don't know who responders are before the alert is created. We create an alert and then decide its destination while sending it. teams: <tmpl_string> was a model which enabled us to determine the teams after the alert is fired but before sending it to opsgenie. We would like to have something like this:

responders:
 - names: <tmpl_string>
   type: teams

bwplotka · 2019-08-02T12:31:10Z

From my side that is a fair request. Do you mind adding an Github issue with those details on AM? We can improve this indeed. (:

anandsinghkunwar · 2019-12-23T10:30:40Z

@hrancic Was an issue filed?
EDIT: It seems it was #1987

bwplotka mentioned this pull request Apr 26, 2019

opsgenie: Move from not supported team field to responders for alert routing. #1818

Closed

bwplotka force-pushed the issue1818 branch from a22daf3 to aa5fd0b Compare April 27, 2019 20:10

bwplotka force-pushed the issue1818 branch 3 times, most recently from 6f8ee98 to 54f633c Compare April 28, 2019 16:17

simonpasquier reviewed Apr 29, 2019

View reviewed changes

bwplotka force-pushed the issue1818 branch 2 times, most recently from e67726c to 90fe1c8 Compare April 30, 2019 20:41

bwplotka force-pushed the issue1818 branch from d865f6f to b0b9fba Compare April 30, 2019 21:54

simonpasquier approved these changes May 2, 2019

View reviewed changes

stuartnelson3 approved these changes May 2, 2019

View reviewed changes

opsgenie: Moved from deprecated, non documented teams to responders f…

ab93bd1

…ield. Teams config option will fail unmarshalling as it is deprecated. Fixes prometheus#1818 Signed-off-by: Bartek Plotka <bwplotka@gmail.com>

bwplotka force-pushed the issue1818 branch from 108bb42 to ab93bd1 Compare May 9, 2019 15:44

simonpasquier merged commit 9ddc5f1 into prometheus:master May 13, 2019

simonpasquier mentioned this pull request May 15, 2019

Update AlertManager configuration page for AM v0.18 prometheus/docs#1354

Merged

bwplotka deleted the issue1818 branch May 16, 2019 20:12

simonpasquier added the changelog/change label Jun 21, 2019

zemek mentioned this pull request Jul 24, 2019

Update opsgenie API docs with responders field prometheus/docs#1402

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

opsgenie: Moved from deprecated, non documented teams to responders field. #1863

opsgenie: Moved from deprecated, non documented teams to responders field. #1863

bwplotka commented Apr 26, 2019 •

edited by simonpasquier

Loading

bwplotka commented Apr 26, 2019

bwplotka commented Apr 27, 2019

simonpasquier left a comment

simonpasquier Apr 29, 2019

bwplotka Apr 29, 2019

simonpasquier Apr 29, 2019

simonpasquier Apr 29, 2019

simonpasquier Apr 29, 2019

bwplotka Apr 29, 2019

simonpasquier Apr 29, 2019

bwplotka commented Apr 30, 2019

simonpasquier left a comment

stuartnelson3 left a comment

simonpasquier commented May 3, 2019

bwplotka commented May 9, 2019

bwplotka commented May 9, 2019

simonpasquier commented May 10, 2019

hrancic commented Jul 30, 2019

simonpasquier commented Jul 30, 2019

hrancic commented Aug 2, 2019

bwplotka commented Aug 2, 2019

hrancic commented Aug 2, 2019 •

edited

Loading

bwplotka commented Aug 2, 2019

hrancic commented Aug 2, 2019 •

edited

Loading

bwplotka commented Aug 2, 2019

anandsinghkunwar commented Dec 23, 2019 •

edited

Loading

opsgenie: Moved from deprecated, non documented teams to responders field. #1863

opsgenie: Moved from deprecated, non documented teams to responders field. #1863

Conversation

bwplotka commented Apr 26, 2019 • edited by simonpasquier Loading

bwplotka commented Apr 26, 2019

bwplotka commented Apr 27, 2019

simonpasquier left a comment

Choose a reason for hiding this comment

simonpasquier Apr 29, 2019

Choose a reason for hiding this comment

bwplotka Apr 29, 2019

Choose a reason for hiding this comment

simonpasquier Apr 29, 2019

Choose a reason for hiding this comment

simonpasquier Apr 29, 2019

Choose a reason for hiding this comment

simonpasquier Apr 29, 2019

Choose a reason for hiding this comment

bwplotka Apr 29, 2019

Choose a reason for hiding this comment

simonpasquier Apr 29, 2019

Choose a reason for hiding this comment

bwplotka commented Apr 30, 2019

simonpasquier left a comment

Choose a reason for hiding this comment

stuartnelson3 left a comment

Choose a reason for hiding this comment

simonpasquier commented May 3, 2019

bwplotka commented May 9, 2019

bwplotka commented May 9, 2019

simonpasquier commented May 10, 2019

hrancic commented Jul 30, 2019

simonpasquier commented Jul 30, 2019

hrancic commented Aug 2, 2019

bwplotka commented Aug 2, 2019

hrancic commented Aug 2, 2019 • edited Loading

bwplotka commented Aug 2, 2019

hrancic commented Aug 2, 2019 • edited Loading

bwplotka commented Aug 2, 2019

anandsinghkunwar commented Dec 23, 2019 • edited Loading

bwplotka commented Apr 26, 2019 •

edited by simonpasquier

Loading

hrancic commented Aug 2, 2019 •

edited

Loading

hrancic commented Aug 2, 2019 •

edited

Loading

anandsinghkunwar commented Dec 23, 2019 •

edited

Loading