notify/opsgenie: log error from OpsGenie API

[simonpasquier]

Ref: https://groups.google.com/d/msg/prometheus-users/3sAQ9kxEgPI/rE-ADUYiBAAJ

Tested against OpsGenie API, the error message looks like this:

cancelling notify retry for \"opsgenie\" due to unrecoverable error: 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.005,\"requestId\":\"610e618e-7138-4898-9c70-c4eef2d68bf8\"}

[bwplotka]

Awesome, I think it makes sense, but I would suggest splitting this retry function (and maybe rename to isRetriable) to keep things clear. What to do you think?

Otherwise LGTM

[bwplotka]

I think we put too many things into this function to just name it retry (:

Can we have isRetriable with what we have previously and then have this:

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))
		}
	}

in Notify if the isRetriable(..) is false?

[simonpasquier]

Hmm, it wouldn't work as-is because we can have errors that shouldn't be retried and for which logging the response body is still useful IMO.

FWIW the current form is consistent with what we have in other places (eg Slack). If anything we should factor out what is common across notifiers.

[stuartnelson3]

looks good to me. maybe it would be good to look at the notifiers now that they're all nicely split out and see what we can simplify, but definitely for another issue/pr.

[simonpasquier]

I'll have a look for the next step.