Fix OpsGenie Teams field

[Tom-Fawcett]

#1061 broke the OpsGenie Teams field.

The structure of this field has in fact changed between v1 and v2.

OpsGenie Documentation

Tests

• No team config set. ✓
• One team value set. ✓
• Multiple team values set. ✓

---

I apologise for missing this in #1061.

[brian-brazil]

This mix of templating, splitting and a list of single-field structs seems odd. Why not keep the config as-is, and split?

[Tom-Fawcett]

Thank you for the review.

I am probably going to show myself up here - but this was the only way I could think of to produce the desired JSON array of objects. ie:

"teams":[{"name":"foo_team"}, {"name":"bar_team"}], 

[josedonizetti]

@Tom-Fawcett Think that instead of having the teams config be a string https://github.com/prometheus/alertmanager/blob/master/config/notifiers.go#L308 on the alertmanager yaml, it should be a list of key values, this way we can also support the team's id, and not only the name.

eg:

teams: 
   - id: 'team_id1'
   - id: 'team_id2'

or

teams:
  - name: 'team_name1'
  - name: 'team_name2'

@brian-brazil what do you think?

[brian-brazil]

The problem with that is that you have to know the number of teams in advance, as we can only template within a field - not across a list.

[stuartnelson3]

Is there a reason we don't want to do this at the config level? It would break configs for users upgrading, so we would have to do another release.

# Teams that the alert will be routed to. The key value must be either id or name.
[ teams: { <id|name>: <tmpl_string>, ... } ]

The docs support either a name or id as the key; do we want to make the assumption for users that they're providing team names?

[josedonizetti]

maybe to avoid a new release, and also fix the problem we can for now do

1. what @stuartnelson3 said We can drop the opsGenieTeam struct and just append a map[string]string{"name": t}
2. only support one team name, for now, I don't like much the strings.Split by comma approach

Then we discuss a better approach for a next release.

[Tom-Fawcett]

Thank you for the feedback.

The use of Split was to try and preserve functionality without changing config.

I can push the suggestion of using map[string]string{"name": t} in place of opsGenieTeam.

Could I please get some further guidance on the race condition.

And a decision on what I should do about config, if I am removing Split.

[stuartnelson3]

Sorry my comment wasn't clear, there is no race condition in the code you've written. @josedonizetti and I suggested the same thing within minutes of eachother, that's what I was referring to.

Only supporting a single team name would also be breaking the interface we present to users, as we explicitly state that it's a comma separated list of teams.

t := strings.Split(n.conf.Teams, ",")
teams := make([]map[string]string, 0, len(t))
for _, name := range t {
	teams = append(teams, map[string]string{"name": tmpl(name)})
}

I would be fine with the above for now.

[josedonizetti]

@Tom-Fawcett could you tackle the tags problem as well?

[Tom-Fawcett]

@stuartnelson3 ah, yes I missed that joke!

The main difference between your example snippet and this PR (after my latest commit), is you split and then use tmpl. That doesn't seem like it would work?

@josedonizetti yep 👍

[stuartnelson3]

Can you update this to []string, as specified in the API docs, and split n.conf.Tags on , and pass that to the create message?

[stuartnelson3]

disregard, #1108 covers it