From bdd91d2639a1461674131de448c1e66bde8c83b2 Mon Sep 17 00:00:00 2001
From: Simon Pasquier <spasquie@redhat.com>
Date: Tue, 23 Jul 2019 09:49:15 +0200
Subject: [PATCH] notify/opsgenie: log error from OpsGenie API (#1965)

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
---
 notify/opsgenie/opsgenie.go      | 28 ++++++++++++++++----------
 notify/opsgenie/opsgenie_test.go | 34 +++++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/notify/opsgenie/opsgenie.go b/notify/opsgenie/opsgenie.go
index a8f3ba37..3e23a435 100644
--- a/notify/opsgenie/opsgenie.go
+++ b/notify/opsgenie/opsgenie.go
@@ -18,11 +18,14 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
+	"io/ioutil"
 	"net/http"
 	"strings"
 
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
+	"github.com/pkg/errors"
 	commoncfg "github.com/prometheus/common/config"
 	"github.com/prometheus/common/model"
 
@@ -85,7 +88,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
 	}
 	defer notify.Drain(resp)
 
-	return n.retry(resp.StatusCode)
+	return n.retry(resp.StatusCode, resp.Body)
 }
 
 // Like Split but filter out empty strings.
@@ -172,7 +175,7 @@ func (n *Notifier) createRequest(ctx context.Context, as ...*types.Alert) (*http
 	apiKey := tmpl(string(n.conf.APIKey))
 
 	if err != nil {
-		return nil, false, fmt.Errorf("templating error: %s", err)
+		return nil, false, errors.Wrap(err, "templating error")
 	}
 
 	var buf bytes.Buffer
@@ -189,14 +192,19 @@ func (n *Notifier) createRequest(ctx context.Context, as ...*types.Alert) (*http
 	return req.WithContext(ctx), true, nil
 }
 
-func (n *Notifier) retry(statusCode int) (bool, error) {
-	// https://docs.opsgenie.com/docs/response#section-response-codes
-	// Response codes 429 (rate limiting) and 5xx are potentially recoverable
-	if statusCode/100 == 5 || statusCode == 429 {
-		return true, fmt.Errorf("unexpected status code %v", statusCode)
-	} else if statusCode/100 != 2 {
-		return false, fmt.Errorf("unexpected status code %v", statusCode)
+func (n *Notifier) retry(statusCode int, body io.Reader) (bool, error) {
+	if statusCode/100 == 2 {
+		return false, nil
 	}
 
-	return false, nil
+	err := errors.Errorf("unexpected status code %v", statusCode)
+	if body != nil {
+		if bs, errRead := ioutil.ReadAll(body); errRead == nil {
+			err = errors.Errorf("%s: %s", err, string(bs))
+		}
+	}
+
+	// https://docs.opsgenie.com/docs/response#section-response-codes
+	// Response codes 429 (rate limiting) and 5xx are potentially recoverable
+	return statusCode/100 == 5 || statusCode == 429, err
 }
diff --git a/notify/opsgenie/opsgenie_test.go b/notify/opsgenie/opsgenie_test.go
index 46be6b1d..fc16c064 100644
--- a/notify/opsgenie/opsgenie_test.go
+++ b/notify/opsgenie/opsgenie_test.go
@@ -14,8 +14,10 @@
 package opsgenie
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/url"
@@ -38,11 +40,41 @@ func TestOpsGenieRetry(t *testing.T) {
 
 	retryCodes := append(test.DefaultRetryCodes(), http.StatusTooManyRequests)
 	for statusCode, expected := range test.RetryTests(retryCodes) {
-		actual, _ := notifier.retry(statusCode)
+		actual, _ := notifier.retry(statusCode, nil)
 		require.Equal(t, expected, actual, fmt.Sprintf("error on status %d", statusCode))
 	}
 }
 
+func TestOpsGenieErr(t *testing.T) {
+	notifier := new(Notifier)
+	for _, tc := range []struct {
+		status   int
+		body     io.Reader
+		expected string
+	}{
+		{
+			status:   http.StatusUnprocessableEntity,
+			body:     nil,
+			expected: "unexpected status code 422",
+		},
+		{
+			status:   http.StatusUnprocessableEntity,
+			body:     bytes.NewBuffer([]byte(`{"message":"Request body is not processable. Please check the errors.","errors":{"priority":"should be one of [ P1, P2, P3, P4, P5 ]"},"took":0.002,"requestId":"865a4f83-99d9-48c8-9550-42a375a3a387"}`)),
+			expected: `unexpected status code 422: {"message":"Request body is not processable. Please check the errors.","errors":{"priority":"should be one of [ P1, P2, P3, P4, P5 ]"},"took":0.002,"requestId":"865a4f83-99d9-48c8-9550-42a375a3a387"}`,
+		},
+		{
+			status:   http.StatusInternalServerError,
+			body:     bytes.NewBuffer([]byte("internal error")),
+			expected: "unexpected status code 500: internal error",
+		},
+	} {
+		t.Run("", func(t *testing.T) {
+			_, err := notifier.retry(tc.status, tc.body)
+			require.Equal(t, err.Error(), tc.expected)
+		})
+	}
+}
+
 func TestOpsGenieRedactedURL(t *testing.T) {
 	ctx, u, fn := test.GetContextWithCancelingURL()
 	defer fn()