diff --git a/notify/webex/webex.go b/notify/webex/webex.go index 949bb614f8..4e2e012b0e 100644 --- a/notify/webex/webex.go +++ b/notify/webex/webex.go @@ -19,6 +19,8 @@ import ( "encoding/json" "log/slog" "net/http" + "strconv" + "time" commoncfg "github.com/prometheus/common/config" @@ -54,7 +56,7 @@ func New(c *config.WebexConfig, t *template.Template, l *slog.Logger, httpOpts . tmpl: t, logger: l, client: client, - retrier: ¬ify.Retrier{}, + retrier: ¬ify.Retrier{RetryCodes: []int{http.StatusTooManyRequests}}, } return n, nil @@ -107,9 +109,35 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) } shouldRetry, err := n.retrier.Check(resp.StatusCode, resp.Body) + // Not deferred: Check has already consumed the body, and the connection must be + // released before the Retry-After wait below rather than held for its duration. + notify.Drain(resp) + if err != nil { + if resp.StatusCode == http.StatusTooManyRequests { + if d := parseRetryAfter(resp.Header.Get("Retry-After")); d > 0 { + logger.Warn("Rate limited by Webex, waiting before retry", "retry_after_secs", d.Seconds()) + select { + case <-time.After(d): + case <-ctx.Done(): + } + } + } return shouldRetry, notify.NewErrorWithReason(notify.GetFailureReasonFromStatusCode(resp.StatusCode), err) } return false, nil } + +// parseRetryAfter parses Retry-After as seconds; 0 if empty, invalid, or non-positive. +// TODO: switch to notify.ParseRetryAfter once upstream #5389 merges a shared helper. +func parseRetryAfter(val string) time.Duration { + if val == "" { + return 0 + } + seconds, err := strconv.Atoi(val) + if err != nil || seconds <= 0 { + return 0 + } + return time.Duration(seconds) * time.Second +} diff --git a/notify/webex/webex_test.go b/notify/webex/webex_test.go index eb12cfc2b6..ef5e2d2315 100644 --- a/notify/webex/webex_test.go +++ b/notify/webex/webex_test.go @@ -49,7 +49,8 @@ func TestWebexRetry(t *testing.T) { ) require.NoError(t, err) - for statusCode, expected := range test.RetryTests(test.DefaultRetryCodes()) { + retryCodes := append(test.DefaultRetryCodes(), http.StatusTooManyRequests) + for statusCode, expected := range test.RetryTests(retryCodes) { actual, _ := notifier.retrier.Check(statusCode, nil) require.Equal(t, expected, actual, "error on status %d", statusCode) } @@ -169,6 +170,88 @@ func TestWebexTemplating(t *testing.T) { } } +func TestWebexRetryAfterSleep(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "1") + w.WriteHeader(http.StatusTooManyRequests) + })) + defer srv.Close() + u, err := url.Parse(srv.URL) + require.NoError(t, err) + + notifier, err := New( + &config.WebexConfig{ + HTTPConfig: &commoncfg.HTTPClientConfig{}, + APIURL: &amcommoncfg.URL{URL: u}, + }, + test.CreateTmpl(t), + promslog.NewNopLogger(), + ) + require.NoError(t, err) + + ctx := notify.WithGroupKey(context.Background(), "1") + alert := &types.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"lbl1": "val1"}, + StartsAt: time.Now(), + EndsAt: time.Now().Add(time.Hour), + }, + } + + start := time.Now() + retry, err := notifier.Notify(ctx, alert) + elapsed := time.Since(start) + + require.True(t, retry) + require.Error(t, err) + require.GreaterOrEqual(t, elapsed, 1*time.Second, "should have waited at least 1 second for Retry-After") +} + +func TestWebexRetryAfterContextCancelled(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "2") + w.WriteHeader(http.StatusTooManyRequests) + })) + defer srv.Close() + u, err := url.Parse(srv.URL) + require.NoError(t, err) + + notifier, err := New( + &config.WebexConfig{ + HTTPConfig: &commoncfg.HTTPClientConfig{}, + APIURL: &amcommoncfg.URL{URL: u}, + }, + test.CreateTmpl(t), + promslog.NewNopLogger(), + ) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + ctx = notify.WithGroupKey(ctx, "1") + + // Cancel context after a short delay to interrupt the Retry-After sleep. + go func() { + time.Sleep(100 * time.Millisecond) + cancel() + }() + + alert := &types.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"lbl1": "val1"}, + StartsAt: time.Now(), + EndsAt: time.Now().Add(time.Hour), + }, + } + + start := time.Now() + retry, err := notifier.Notify(ctx, alert) + elapsed := time.Since(start) + + require.True(t, retry) + require.Error(t, err) + require.Less(t, elapsed, 2*time.Second, "should not have waited the full Retry-After duration") +} + func TestWebexFailureReason(t *testing.T) { for _, tc := range []struct { name string