Fix OpsGenie notifier and add unit tests

See prometheus#1223, looks like OpsGenie now sometimes returns a 422 when you
don't specify a team. This change cleans up the JSON output and
add a few unit tests.

notify/impl.go

@@ -952,9 +952,39 @@ type opsGenieCloseMessage struct {
 
 // Notify implements the Notifier interface.
 func (n *OpsGenie) Notify(ctx context.Context, as ...*types.Alert) (bool, error) {
+	req, retry, err := n.createRequest(ctx, as...)
+	if err != nil {
+		return retry, err
+	}
+
+	resp, err := ctxhttp.Do(ctx, http.DefaultClient, req)
+
+	if err != nil {
+		return true, err
+	}
+	defer resp.Body.Close()
+
+	return n.retry(resp.StatusCode)
+}
+
+// Like Split but filter out empty strings.
+func safeSplit(s string, sep string) []string {
+	// https://github.com/golang/go/wiki/SliceTricks#filtering-without-allocating
+	a := strings.Split(strings.TrimSpace(s), sep)
+	b := a[:0]
+	for _, x := range a {
+		if x != "" {
+			b = append(b, x)
+		}
+	}
+	return b
+}
+
+// Create requests for a list of alerts.
+func (n *OpsGenie) createRequest(ctx context.Context, as ...*types.Alert) (*http.Request, bool, error) {
 	key, ok := GroupKey(ctx)
 	if !ok {
-		return false, fmt.Errorf("group key missing")
+		return nil, false, fmt.Errorf("group key missing")
 	}
 	data := n.tmpl.Data(receiverName(ctx, n.logger), groupLabels(ctx, n.logger), as...)
 
@@ -987,45 +1017,39 @@ func (n *OpsGenie) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
 
 		apiURL = n.conf.APIURL + "v2/alerts"
 		var teams []map[string]string
-		for _, t := range strings.Split(string(tmpl(n.conf.Teams)), ",") {
+		for _, t := range safeSplit(string(tmpl(n.conf.Teams)), ",") {
 			teams = append(teams, map[string]string{"name": t})
 		}
+		tags := safeSplit(string(tmpl(n.conf.Tags)), ",")
+
 		msg = &opsGenieCreateMessage{
 			Alias:       alias,
 			Message:     message,
 			Description: tmpl(n.conf.Description),
 			Details:     details,
 			Source:      tmpl(n.conf.Source),
 			Teams:       teams,
-			Tags:        strings.Split(string(tmpl(n.conf.Tags)), ","),
+			Tags:        tags,
 			Note:        tmpl(n.conf.Note),
 			Priority:    tmpl(n.conf.Priority),
 		}
 	}
 	if err != nil {
-		return false, fmt.Errorf("templating error: %s", err)
+		return nil, false, fmt.Errorf("templating error: %s", err)
 	}
 
 	var buf bytes.Buffer
 	if err := json.NewEncoder(&buf).Encode(msg); err != nil {
-		return false, err
+		return nil, false, err
 	}
 
 	req, err := http.NewRequest("POST", apiURL, &buf)
 	if err != nil {
-		return true, err
+		return nil, true, err
 	}
 	req.Header.Set("Content-Type", contentTypeJSON)
 	req.Header.Set("Authorization", fmt.Sprintf("GenieKey %s", n.conf.APIKey))
-
-	resp, err := ctxhttp.Do(ctx, http.DefaultClient, req)
-
-	if err != nil {
-		return true, err
-	}
-	defer resp.Body.Close()
-
-	return n.retry(resp.StatusCode)
+	return req, true, nil
 }
 
 func (n *OpsGenie) retry(statusCode int) (bool, error) {

notify/impl_test.go

@@ -4,8 +4,18 @@ import (
 	"fmt"
 	"net/http"
 	"testing"
+	"time"
 
+	"github.com/go-kit/kit/log"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/net/context"
+
+	"github.com/prometheus/common/model"
+	"github.com/prometheus/alertmanager/types"
+	"github.com/prometheus/alertmanager/config"
+	"github.com/prometheus/alertmanager/template"
+	"net/url"
+	"io/ioutil"
 )
 
 func TestWebhookRetry(t *testing.T) {
@@ -61,6 +71,7 @@ func TestWechatRetry(t *testing.T) {
 		require.Equal(t, expected, actual, fmt.Sprintf("error on status %d", statusCode))
 	}
 }
+
 func TestOpsGenieRetry(t *testing.T) {
 	notifier := new(OpsGenie)
 
@@ -181,3 +192,78 @@ func defaultRetryCodes() []int {
 		http.StatusNetworkAuthenticationRequired,
 	}
 }
+
+func createTmpl(t *testing.T) *template.Template {
+	tmpl, err := template.FromGlobs()
+	require.NoError(t, err)
+	tmpl.ExternalURL, _ = url.Parse("http://am")
+	return tmpl
+}
+
+func readBody(t *testing.T,  r *http.Request) string {
+	body, err := ioutil.ReadAll(r.Body)
+	require.NoError(t, err)
+	return string(body)
+}
+
+func TestOpsGenie(t *testing.T) {
+	logger := log.NewNopLogger()
+	tmpl := createTmpl(t)
+	conf := &config.OpsGenieConfig{
+		NotifierConfig: config.NotifierConfig{
+			VSendResolved: true,
+		},
+		Message:     `{{ .CommonLabels.Message }}`,
+		Description: `{{ .CommonLabels.Description }}`,
+		Source:      `{{ .CommonLabels.Source }}`,
+		Teams:        `{{ .CommonLabels.Teams }}`,
+		Tags:        `{{ .CommonLabels.Tags }}`,
+		Note:        `{{ .CommonLabels.Note }}`,
+		Priority:    `{{ .CommonLabels.Priority }}`,
+		APIKey:      `s3cr3t`,
+		APIURL:      `https://opsgenie/api`,
+	}
+	notifier := NewOpsGenie(conf, tmpl, logger)
+
+	ctx := context.Background()
+	ctx = WithGroupKey(ctx, "1")
+
+	expectedUrl, _ := url.Parse("https://opsgenie/apiv2/alerts")
+
+	// Empty alert.
+	alert1:= &types.Alert{
+			Alert: model.Alert{
+				StartsAt: time.Now(),
+				EndsAt: time.Now().Add(time.Hour),
+			},
+	}
+	expectedBody := `{"alias":"6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b","message":"","details":{},"source":""}
+`
+	req, retry, err := notifier.createRequest(ctx, alert1)
+	require.NoError(t, err)
+	require.Equal(t, true, retry)
+	require.Equal(t, expectedUrl, req.URL)
+	require.Equal(t, "GenieKey s3cr3t", req.Header.Get("Authorization"))
+	require.Equal(t, expectedBody, readBody(t, req))
+
+	// Fully defined alert.
+	alert2:= &types.Alert{
+		Alert: model.Alert{
+			Labels: model.LabelSet{
+				"Message": "message",
+				"Description": "description",
+				"Source": "http://prometheus",
+				"Teams": "TeamA,TeamB",
+				"Tags": "tag1,tag2",
+				"Note": "this is a note",
+				"Priotity": "P1",
+				},
+			StartsAt: time.Now(),
+			EndsAt: time.Now().Add(time.Hour),
+		},
+	}
+	expectedBody = `{"alias":"6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b","message":"message","description":"description","details":{},"source":"http://prometheus","teams":[{"name":"TeamA"},{"name":"TeamB"}],"tags":["tag1","tag2"],"note":"this is a note"}
+`
+	req, retry, err = notifier.createRequest(ctx, alert2)
+	require.Equal(t, expectedBody, readBody(t, req))
+}